[RFC] Apply our brand colors on source{d} charts #241

dpordomingo · 2019-08-07T09:52:03Z

This PR was superseded by #257, #258 and #259 as agreed at #241 (comment)

disclaimer: This PR applies our branding on current source{d} charts. It is done on different levels, as it can be seen in each commit. We can use this PR to discuss the process or do it in separate and sequential PRs. The proposed palettes should be approved or amended by @src-d/product

List of changes:

3c4c93c Add a new theme with 4 new palettes with colors from source{d} Brand Style Guide
9889bf4 2f06d2c fe18682 Make srcdMain the default palette for SUPERSET_DEFAULT (instead of the current bnbColors), and use it also as the default one when creating new charts.
f758a82 eadfeb3 Assign our palettes on current charts, reassigning some palettes on the current charts as described below.

The final result can also be seen in the overview screenshot and in the collaboration screenshot.

TODO

I found some lines of code in superset still using original palettesbnbColors, d3Category10 and blue_white_yellow, but since it does not have impact in our current dashboards (as far as I could validate in my tests, and can be seen in the screenshots below), I think that it could be addressed in a separate PR.

Available palettes

These are the four new palettes (plus the current ones)

dpordomingo · 2019-08-07T09:52:36Z

Palette reassignments

Changes applied on overview dashboard:

chart	old palette	new palette
Number of releases per month and repository	`bnbColors`	`SUPERSET_DEFAULT`
Commits Evolution	`bnbColors`	`srcdRoyal`
Number of commits per day of the week	`bnbColors`	`srcdRoyal`
Number of commits to HEAD per month	`d3Category10`	`srcdRoyal`
Total number of commits per repository	`SUPERSET_DEFAULT`	`srcdRoyal`

Changes applied on collaboration dashboard:

chart	old palette	new palette
Most Active Repositories	`bnbColors`	`srcdOpaques`
Number of pull requests created	`bnbColors`	`srcdRoyal`
Time to Merge Change Over Time	`bnbColors`	`srcdRoyal`
Breakdown by Pull Request Outcome	`bnbColors`	`SUPERSET_DEFAULT`

dpordomingo · 2019-08-07T09:53:07Z

overview dashboard

click to see the palettes applied on each chart of overview dashboard

SUPERSET_DEFAULT
- Languages
- Number of releases per month and repository
- Number of commits per month breakdown per repo
- Number of files per languages
- Number of lines of code per language
- Number of contributions per organization
- Organization wordcloud
- Number of contributions per email
- Email wordcloud
srcdRoyal
- Commits Evolution
- Number of commits per day of the week
- Number of commits to HEAD per month
- Total number of commits per repository
NONE
- Repository Count
- Number of collaborators
- Repositories by number of files
- Repository by number of collaborators
- Compliance score
- Missing files
- Files missing summary

dpordomingo · 2019-08-07T09:53:32Z

collaboration dashboard

note: Pull Requests Workflow chart could not be customized because its underlying type (Event Flow) does not allow to change its visual properties.

click to see the palettes applied on each chart of collaboration dashboard

SUPERSET_DEFAULT
- Breakdown by Pull Request Outcome
srcdRoyal
- Number of pull requests created
- Time to Merge Change Over Time
srcdOpaques
- Most Active Repositories
NONE
- Not Merged Pull Requests
- Avg. Time to Merge
- Median Time to Merge
- Pull Requests Workflow

se7entyse7en · 2019-08-07T10:10:16Z

srcd/dashboards/gitbase/overview.json

@@ -14,7 +14,7 @@
                            "datasource_name": "gitbase.admin admin-commit_merges-8X2HBrQre",
                            "datasource_type": "table",
                            "id": 9,
-                            "params": "{\"adhoc_filters\": [], \"annotation_layers\": [], \"bottom_margin\": \"auto\", \"color_scheme\": \"bnbColors\", \"comparison_type\": \"values\", \"contribution\": false, \"datasource\": \"14__table\", \"granularity_sqla\": \"commit_author_when\", \"groupby\": [\"commit_type\"], \"line_interpolation\": \"linear\", \"metrics\": [{\"aggregate\": null, \"column\": null, \"expressionType\": \"SQL\", \"fromFormData\": true, \"hasCustomLabel\": false, \"label\": \"COUNT(*)\", \"optionName\": \"metric_n4xn9emh55_60hid92bszp\", \"sqlExpression\": \"COUNT(*)\"}], \"order_desc\": true, \"resample_fillmethod\": null, \"resample_how\": null, \"resample_rule\": null, \"rich_tooltip\": true, \"rolling_type\": \"None\", \"row_limit\": 50000, \"show_brush\": \"auto\", \"show_controls\": false, \"show_legend\": true, \"slice_id\": 4, \"stacked_style\": \"stack\", \"time_grain_sqla\": \"P1M\", \"time_range\": \"100 years ago : \", \"timeseries_limit_metric\": null, \"viz_type\": \"area\", \"x_axis_format\": \"smart_date\", \"x_axis_label\": \"\", \"x_axis_showminmax\": false, \"x_ticks_layout\": \"auto\", \"y_axis_bounds\": [null, null], \"y_axis_format\": \".3s\", \"y_log_scale\": false, \"remote_id\": \"be4154a6-726c-4dcc-bcff-da20558aaa55\", \"datasource_name\": \"gitbase.admin admin-commit_merges-8X2HBrQre\", \"schema\": \"gitbase.admin admin-commit_merges-8X2HBrQre\", \"database_name\": \"gitbase\"}",
+                            "params": "{\"adhoc_filters\": [], \"annotation_layers\": [], \"bottom_margin\": \"auto\", \"color_scheme\": \"srcdRoyal\", \"comparison_type\": \"values\", \"contribution\": false, \"datasource\": \"14__table\", \"granularity_sqla\": \"commit_author_when\", \"groupby\": [\"commit_type\"], \"line_interpolation\": \"linear\", \"metrics\": [{\"aggregate\": null, \"column\": null, \"expressionType\": \"SQL\", \"fromFormData\": true, \"hasCustomLabel\": false, \"label\": \"COUNT(*)\", \"optionName\": \"metric_n4xn9emh55_60hid92bszp\", \"sqlExpression\": \"COUNT(*)\"}], \"order_desc\": true, \"resample_fillmethod\": null, \"resample_how\": null, \"resample_rule\": null, \"rich_tooltip\": true, \"rolling_type\": \"None\", \"row_limit\": 50000, \"show_brush\": \"auto\", \"show_controls\": false, \"show_legend\": true, \"slice_id\": 4, \"stacked_style\": \"stack\", \"time_grain_sqla\": \"P1M\", \"time_range\": \"100 years ago : \", \"timeseries_limit_metric\": null, \"viz_type\": \"area\", \"x_axis_format\": \"smart_date\", \"x_axis_label\": \"\", \"x_axis_showminmax\": false, \"x_ticks_layout\": \"auto\", \"y_axis_bounds\": [null, null], \"y_axis_format\": \".3s\", \"y_log_scale\": false, \"remote_id\": \"be4154a6-726c-4dcc-bcff-da20558aaa55\", \"datasource_name\": \"gitbase.admin admin-commit_merges-8X2HBrQre\", \"schema\": \"gitbase.admin admin-commit_merges-8X2HBrQre\", \"database_name\": \"gitbase\"}",


Not related to this PR, but it came to my mind now.

@smacker is this the result of a json.dumps(params) during export? If it is so, do you think that it could make sense to replace it with a json.dumps(params, indent=4) where params is an OrderedDict? It should simplify the diff as each key would be in different rows.

I just edited current dashboards, but I agree with you @se7entyse7en; diffs would be better if we could have better formatted this section.

I just edited current dashboards

Yup, I imagined that, and thanks for posting the screenshots! 👍

This is just a value from a database column which can be either empty string or serialized json. In theory, we can amend import/export to serialize it differently. But I wouldn't hack import/export anymore until apache/superset#7829 is merged into upstream, released and merged into our fork.

Okay! Thanks for the info! 👍

ricardobaeta

This is so awesome @dpordomingo ✨ Thank you so much.

smacker · 2019-08-07T11:14:00Z

Eslint fails

dpordomingo · 2019-08-07T14:18:48Z

Now it passes again; thanks @smacker for noticing.

srcd/superset/assets/src/customization/srcdColors.jsx

This commit includes for new palettes for source{d} theme: - srcdMain, with the source{d} main colors from the branding docs. - srcdAll, with: - all colors from srcdMain - all other extra colors defined by the branding docs - one extra color from the gradients between main colors - srcdOpaques, with the six main source{d} colors. - srcdRoyal, with source{d} royal and source{d} deep royal Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

Superset is using 'SUPERSET_DEFAULT' as the default CategoricalScheme key i.e. https://github.com/apache-superset/superset-ui/blob/master/packages/superset-ui-color/src/CategoricalSchemeRegistrySingleton.ts This commit assigns 'srcdMain' palette for 'SUPERSET_DEFAULT' key Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

It overrides the 'bnbColors' as defined by superset. Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

It will select 'srcdMain' palette from the 'Color Scheme' selector when creating a chart (instead of the current default: 'bnbColors') It does not change current selected schemes, but it will affect only the new charts if no other color is manually asigned. Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

Instead of using 'bnbColors' or 'd3Category10' in our charts, it will be used the default color_scheme: 'SUPERSET_DEFAULT' Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

dpordomingo · 2019-08-08T07:57:19Z

~~Since all comments were positive, and our beloved designer already approved it, I think we could turn this RFC into a real PR, and merge it if you agree Carlos~~

dpordomingo · 2019-08-08T08:58:18Z

After a call with @ricardobaeta we agree on splitting this PR in two:

Use SUPERSET_DEFAULT as the default palette #257 Use reduced palettes #258 the logical thing, without visual impact → use defaults instead of hardcoded palettes,
Apply our brand colors on source{d} dashboards #259 the visual part → changing the defaults to use our palette instead of superset ones.

This way, we could merge the first one without visual impact, and iterate later on the visual part, creating more curated palettes and documenting them properly.

carlosms

Code LGTM, up to you to create new PRs or merge this one.

ricardobaeta

As agreed on Slack Call with @dpordomingo we're splitting this PR in two. My only request is to address just that.

Great work ✨

dpordomingo · 2019-08-15T07:51:08Z

This PR was superseded by #257, #258 and #259 as agreed at #241 (comment)

Thanks for your contributions, which let me improve the proposed change.

dpordomingo added the enhancement New feature or request label Aug 7, 2019

dpordomingo requested review from ricardobaeta and a team August 7, 2019 09:52

dpordomingo self-assigned this Aug 7, 2019

dpordomingo force-pushed the palette branch from ca59e43 to dabc8ed Compare August 7, 2019 09:55

se7entyse7en reviewed Aug 7, 2019

View reviewed changes

ricardobaeta approved these changes Aug 7, 2019

View reviewed changes

se7entyse7en approved these changes Aug 7, 2019

View reviewed changes

dpordomingo force-pushed the palette branch from 4b8c0e3 to fc425d1 Compare August 7, 2019 14:00

dpordomingo requested a review from smacker August 7, 2019 14:19

smacker approved these changes Aug 7, 2019

View reviewed changes

smacker reviewed Aug 7, 2019

View reviewed changes

srcd/superset/assets/src/customization/srcdColors.jsx Outdated Show resolved Hide resolved

dpordomingo added 6 commits August 7, 2019 22:17

Make SUPERSET_DEFAULT the default palett for CategoricalScheme

fe18682

It overrides the 'bnbColors' as defined by superset. Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

Set default color_scheme in our charts

f758a82

Instead of using 'bnbColors' or 'd3Category10' in our charts, it will be used the default color_scheme: 'SUPERSET_DEFAULT' Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

Use simpler palettes for charts with only a few metrics

eadfeb3

Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>

dpordomingo force-pushed the palette branch from fc425d1 to eadfeb3 Compare August 7, 2019 20:17

dpordomingo changed the title ~~[RFC] Apply our brand colors on source{d} charts~~ Apply our brand colors on source{d} charts Aug 8, 2019

dpordomingo changed the title ~~Apply our brand colors on source{d} charts~~ [RFC] Apply our brand colors on source{d} charts Aug 8, 2019

carlosms approved these changes Aug 8, 2019

View reviewed changes

ricardobaeta suggested changes Aug 8, 2019

View reviewed changes

ricardobaeta mentioned this pull request Aug 14, 2019

source{d} color scheme by default on Charts Visual Properties #255

Closed

This was referenced Aug 15, 2019

Use SUPERSET_DEFAULT as the default palette #257

Merged

Use reduced palettes #258

Merged

Apply our brand colors on source{d} dashboards #259

Merged

dpordomingo closed this Aug 15, 2019

dpordomingo deleted the palette branch August 15, 2019 09:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RFC] Apply our brand colors on source{d} charts #241

[RFC] Apply our brand colors on source{d} charts #241

dpordomingo commented Aug 7, 2019 •

edited

Loading

dpordomingo commented Aug 7, 2019 •

edited

Loading

dpordomingo commented Aug 7, 2019 •

edited

Loading

dpordomingo commented Aug 7, 2019

se7entyse7en Aug 7, 2019

dpordomingo Aug 7, 2019

se7entyse7en Aug 7, 2019 •

edited

Loading

smacker Aug 7, 2019

se7entyse7en Aug 7, 2019

ricardobaeta left a comment

smacker commented Aug 7, 2019

dpordomingo commented Aug 7, 2019

dpordomingo commented Aug 8, 2019 •

edited

Loading

dpordomingo commented Aug 8, 2019 •

edited

Loading

carlosms left a comment

ricardobaeta left a comment

dpordomingo commented Aug 15, 2019 •

edited

Loading

[RFC] Apply our brand colors on source{d} charts #241

[RFC] Apply our brand colors on source{d} charts #241

Conversation

dpordomingo commented Aug 7, 2019 • edited Loading

TODO

Available palettes

dpordomingo commented Aug 7, 2019 • edited Loading

Palette reassignments

dpordomingo commented Aug 7, 2019 • edited Loading

overview dashboard

dpordomingo commented Aug 7, 2019

collaboration dashboard

se7entyse7en Aug 7, 2019

Choose a reason for hiding this comment

dpordomingo Aug 7, 2019

Choose a reason for hiding this comment

se7entyse7en Aug 7, 2019 • edited Loading

Choose a reason for hiding this comment

smacker Aug 7, 2019

Choose a reason for hiding this comment

se7entyse7en Aug 7, 2019

Choose a reason for hiding this comment

ricardobaeta left a comment

Choose a reason for hiding this comment

smacker commented Aug 7, 2019

dpordomingo commented Aug 7, 2019

dpordomingo commented Aug 8, 2019 • edited Loading

dpordomingo commented Aug 8, 2019 • edited Loading

carlosms left a comment

Choose a reason for hiding this comment

ricardobaeta left a comment

Choose a reason for hiding this comment

dpordomingo commented Aug 15, 2019 • edited Loading

dpordomingo commented Aug 7, 2019 •

edited

Loading

dpordomingo commented Aug 7, 2019 •

edited

Loading

dpordomingo commented Aug 7, 2019 •

edited

Loading

se7entyse7en Aug 7, 2019 •

edited

Loading

dpordomingo commented Aug 8, 2019 •

edited

Loading

dpordomingo commented Aug 8, 2019 •

edited

Loading

dpordomingo commented Aug 15, 2019 •

edited

Loading