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

✨ (entity selector) keep selected entities in options list #4035

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sophiamersmann
Copy link
Member

@sophiamersmann sophiamersmann commented Oct 8, 2024

Resolves #3920

Keeps selected and unselected entities both in the list of entities to choose from.

I didn't remove the animation library because the animations make it look a bit smoother when the list of selected entities is in view.

Browsers like Chrome and Firefox are smart enough not to do a layout shift when a new entity is selected while the list of selected entities is not currently in view. But in other browsers like Safari, selecting a new entity always leads to a layout shift. The animation also helps here to make the layout shift a bit less jarring.

Marcel's point below is addressed :)

Copy link
Member Author

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

Join @sophiamersmann and the rest of your teammates on Graphite Graphite

@owidbot
Copy link
Contributor

owidbot commented Oct 8, 2024

Quick links (staging server):

Site Admin Wizard

Login: ssh owid@staging-site-entity-selector-animation

SVG tester:

Number of differences (default views): 2773 (420be5) ❌
Number of differences (all views): 1625 (78e72a) ❌

Edited: 2024-10-14 12:35:17 UTC
Execution time: 1.28 seconds

@marcelgerber
Copy link
Member

CleanShot.2024-10-09.at.14.23.35.mp4

One thing I did notice here is that the checkboxes are behaving weirdly when using keyboard navigation (i.e. tab and space).
Activation works fine, but they are at first still unchecked and only properly become checked once the next checkbox is toggled.

I'm seeing this in both Edge and Firefox.

@sophiamersmann
Copy link
Member Author

sophiamersmann commented Oct 9, 2024

This is just a draft implementation – I just changed as few lines as possible to get a staging site I could share.

If the problem you're describing also exists on prod, then probably worth opening an issue for it.

I'll keep this in mind though when I'm working on it :)

@sophiamersmann sophiamersmann force-pushed the entity-selector-animation branch 3 times, most recently from 2d6b8d7 to 22f19f0 Compare October 11, 2024 10:12
@sophiamersmann sophiamersmann marked this pull request as ready for review October 11, 2024 10:24
Copy link
Member

@marcelgerber marcelgerber left a comment

Choose a reason for hiding this comment

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

Nice, and great that we can get rid of a load of code! 🐰

I found a few small things, but great work otherwise :)

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.

Entity selector: don't remove selected entities from the full list
3 participants