🐛 fix global entity selector

[sophiamersmann]

Fixes #3931

The global entity selector manages the selection of a chart from outside by passing in its own selection object, which should take precedence over the config's selectedEntityNames.

In #3793, Grapher's updateFromObject method was updated to also change the current selection if selectedEntityNames is given. That's sensible (and is needed in the admin), but updateFromObject is also called in the constructor. If selectedEntityNames and selection both are given then selectedEntityNames always won, which broke the global entity selector (since it relies on selection being respected).

I think it's sensible to apply the passed-in selection only if it's non-empty, but this leads to funny behaviour of the global entity selector if the selection happens to be empty (e.g., http://staging-site-fix-global-entity-selector/energy/country/spain?country=). But, apparently, even before #3793, the global entity selector has behaved weirdly for empty selections (e.g., https://e7fd384b.owid.pages.dev/energy/country/spain?country=). It's not ideal, but I don't think it's too bad.

[sophiamersmann]

• 🐛 (admin) keep selection when exporting mobile version #3964
• 🐛 fix global entity selector #3963 👈
• master

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

Join @sophiamersmann and the rest of your teammates on Graphite

[owidbot]

Quick links (staging server):

Site	Admin	Wizard

Login: ssh owid@staging-site-fix-global-entity-selector

• Site-screenshots: https://github.com/owid/site-screenshots/compare/fix-global-entity-selector
• SVG tester: https://github.com/owid/owid-grapher-svgs/compare/fix-global-entity-selector

SVG tester:

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

Edited: 2024-09-16 13:52:57 UTC
Execution time: 1.25 seconds

[ikesau]

Nice! Fixes the issue 👌

I think that the empty query param thing is only be an issue if someone gets linked to it that way.

If the user did it to themselves by unselecting the selected country, then I think they would understand why the charts aren't rendering properly.

So maybe, if you care enough, you could prune the empty ?country= query param when the page mounts, before the embedder has started doing anything, and I think that should be fine because http://staging-site-fix-global-entity-selector/energy/country/new-zealand works?

Feel free to merge without that, though 🙂

[sophiamersmann]

Yeah, that's a good suggestion, thank you, Ike! If I have time this week, I'll follow up with another PR, but I'll merge as-is for now to get the bug fixed.

[sophiamersmann]

Merge activity

• Sep 17, 5:46 AM EDT: @sophiamersmann started a stack merge that includes this pull request via Graphite.
• Sep 17, 5:46 AM EDT: @sophiamersmann merged this pull request with Graphite.