🔨 remove default layer from chart configs / TAS-675

[sophiamersmann]

Removes the default layer from the full config of charts.

This is the smaller version that removes the default layer from the full configs of all charts but keeps using it in the admin. If I find the time this week, I also want to remove the admin's dependence on the default config, but even if I don't find the time, I think this PR is okay to be merged as is since keeping the default config up-to-date is zero effort (we have a GitHub action that infers the default config from the schema and automatically commits it).

I tested this by comparing the previously baked Grapher configs with the now-baked Grapher configs. The diff was empty for most configs. There were a few that used to have map.colorScale set to an empty object (not sure why) that is missing now, but that makes no difference to Grapher.

The SVG tester differences are due to a parent PR.

[sophiamersmann]

• ✨ (grapher) support multiple chart types / TAS-694 #4140
• 🔨 use fixed defaultGrapherConfig in migrations #4113
• 🔨 (grapher) turn related questions into an object / TAS-676 #4108
• 🔨 remove default layer from chart configs / TAS-675 #4103 👈 (View in Graphite)
• ✨ (line chart) set y-axis min default to 0 #4080
• 🐛 (explorer) merge configs correctly #4091
• ✨ log explorer views to google analytics #4098
• ✨ log grapher views to google analytics #4076
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[owidbot]

Quick links (staging server):

Site	Admin	Wizard	Docs

Login: ssh owid@staging-site-remove-default-layer

• Site-screenshots: https://github.com/owid/site-screenshots/compare/remove-default-layer
• SVG tester: https://github.com/owid/owid-grapher-svgs/compare/remove-default-layer

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2024-11-11 17:17:25 UTC
Execution time: 1.24 seconds

[danyx23]

I like this! I have found one issue though and I'm not sure if this is because the admin does things differently now or if this is just a little bug or if I misunderstand how this is supposed to work.

Here is what I did:

• take a line chart (like this one) that does not currently have inheritance
• create a parent admin layer chart
• set a property to a non-default value (e.g. y column max) and save
• in the original standalone chart, refresh and check that the parent is now set
• go to the customize tab and click "bind to data" under y column max

I would expect the parent value to be in effect now but it is the default value. Am I wrong or is this a bug?

[sophiamersmann]

"Bind to data" means the y-column max is inferred from data, it essentially sets yAxis.max to "auto". I haven't tried but the behaviour sounds correct to me.

[sophiamersmann]

The label is maybe a bit confusing – maybe "Unset" would be clearer?

[notion-workspace]

Remove default layer from full configs

[sophiamersmann]

@danyx23 do you think you have time to finish this review in the next week or so? I think it would be good if my changes and especially Marcel‘s work on narrative views builds on this.

[danyx23]

Hey @sophiamersmann, thanks for the ping! (btw please ping me earlier next time, I don't get a notification if you reply here without mentioning me :)). I played with it and it looks good! The only thing that would need to be changed now is that Martin has merged his work to add chart_config entries for Mdim views and they would also need to be stripped of the defaults for completeness. I think it shouldn't be much work and it would be good to include this in this PR but if you think this is too much work now then it's also fine to merge this as is and do the mdim work in cooldown.

[sophiamersmann]

Thanks! Yes, I'll rebase & update the PR

[sophiamersmann]

@danyx23, as far as I can see, the defaults haven't been merged into the mdim view configs. Am I missing something?

[danyx23]

@sophiamersmann Ah interesting - that was sort of a bug in that work then that now is a feature :). Yes, then there is nothing left to do in that regard and this can go.