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

enhance(admin): persist entities when changing the chart type #2702

Merged
merged 2 commits into from
Oct 20, 2023

Conversation

sophiamersmann
Copy link
Member

Problem

  • When changing chart types (or adding y-dimensions), a random sample of entities are selected
  • Apparently, that's annoying for authors; they would prefer to have entities persisted if possible (see Slack)

Solution

  • Only sample entities when none are selected

if (entity) selection.setSelectedEntities([entity])
// non-stacked charts with multiple y-dimensions should select a single entity by default.
// if possible, the currently selected entity is persisted, otherwise "World" is preferred
if (selection.numSelectedEntities !== 1) {
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also pick the first entity if multiple entities are currently selected

)
// stacked charts or charts with a single y-dimension should select multiple entities by default.
// if possible, the currently selected entities are persisted, otherwise a random sample is selected
if (selection.numSelectedEntities === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

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

What if the numSelectedEntities is 1? I've chosen to persist it but we could also sample additional entities...

@github-actions
Copy link

This PR has had no activity within the last two weeks. It is considered stale and will be closed in 3 days if no further activity is detected.

@github-actions github-actions bot added the stale label Oct 20, 2023
@sophiamersmann sophiamersmann force-pushed the fix-admin-persist-entitties branch from 50341f2 to a5d47b3 Compare October 20, 2023 10:14
@sophiamersmann
Copy link
Member Author

it's a low risk change, so I'll just merge this in now :)

@sophiamersmann sophiamersmann merged commit dfd0cf7 into master Oct 20, 2023
13 checks passed
@sophiamersmann sophiamersmann deleted the fix-admin-persist-entitties branch October 20, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant