Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 take table filter toggle into account for data download / TAS-796 #4433

Merged
merged 2 commits into from
Jan 14, 2025

Conversation

sophiamersmann
Copy link
Member

@sophiamersmann sophiamersmann commented Jan 10, 2025

Fixes #4432

If 'Show selection only' on the table tab is toggled, then 'Download displayed data' only downloads data for those entities that are displayed in the table.

If 'Show selection only' isn't toggled, then 'Download displayed data' downloads all data (since all data is displayed). Don't know if this could be confusing. I find it okay.

This PR makes non-trivial changes to the Grapher component. Let me know if you prefer to wait for your refactor before merging this PR.

👇🏻 See my comment about grapher.inputTable vs grapher.table below


This is part 1 of 2 in a stack made with GitButler:

@owidbot
Copy link
Contributor

owidbot commented Jan 10, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-fix-filtered-data-download

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 220 (3d40b3) ❌

Edited: 2025-01-14 12:01:12 UTC
Execution time: 1.23 seconds

@sophiamersmann sophiamersmann force-pushed the fix-filtered-data-download branch from 525b08c to a67c9c0 Compare January 10, 2025 11:36
@sophiamersmann sophiamersmann force-pushed the fix-filtered-data-download branch from a67c9c0 to 7d33c7f Compare January 10, 2025 11:46
@sophiamersmann sophiamersmann marked this pull request as ready for review January 10, 2025 12:40
@sophiamersmann sophiamersmann changed the title 🐛 take table filter toggle into account for data download 🐛 take table filter toggle into account for data download / TAS-796 Jan 10, 2025
Copy link

Copy link
Contributor

@danyx23 danyx23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Feel free to merge already, the grapher state refactor will still be open for a few days and I'm pretty used to dealing with rebase conflicts by now 😬

@sophiamersmann sophiamersmann force-pushed the fix-filtered-data-download branch from 7d33c7f to fcab837 Compare January 14, 2025 11:34
@sophiamersmann sophiamersmann force-pushed the fix-filtered-data-download branch from fcab837 to 7d02902 Compare January 14, 2025 11:44
@sophiamersmann sophiamersmann merged commit d975d8c into master Jan 14, 2025
20 of 22 checks passed
@sophiamersmann sophiamersmann deleted the fix-filtered-data-download branch January 14, 2025 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Download of 'displayed data' should take filtered table into account
3 participants