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.

Sign up for GitHub

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

Jump to bottom

replace {{did-insert}} modifiers in LocaleChooser component #7922

Merged
merged 6 commits into from
Jul 8, 2024
Merged

replace {{did-insert}} modifiers in LocaleChooser component #7922

merged 6 commits into from
Jul 8, 2024

Conversation

stopfstedt
Copy link
Member

@stopfstedt stopfstedt commented Jul 2, 2024 •
edited
Loading

refs ilios/ilios#5374

an alternative solution to #7919

@michaelchadwick
Copy link
Contributor

I nominate your alternative solution after our discussion.

@jrjohnson
Copy link
Member

Oh this is nice, I was originally thinking we'd have to go with @michaelchadwick's solution in a bunch of places where we use did-insert because it's actually hooked up to the DOM so it makes sense. This works very well though, especially the focus on index like that a lot.

@stopfstedt stopfstedt mentioned this pull request Jul 5, 2024
Closed
@stopfstedt stopfstedt marked this pull request as ready for review July 5, 2024 16:44
@stopfstedt
focus on locale picker toggle button on locale change without trackin…
a470604 
…g that element.
@stopfstedt
make selector input optional on has-focus helper.
d57d969 
sometimes, the given element is all we need.
@stopfstedt
focus the toggle button on locale-change without the need to track th…
5ee8c97 
…at element.

we have full control over this component's markup, so let's traverse it
to markup from the click target to the toggle button,
instead of tracking the toggle button separately.
one less modifier.
@stopfstedt
replace did-insert modifier with focus modifier.
c7220ff 
drop-in replacement.
@stopfstedt
replace {{autofocus}} modifier with {{focus}}. rm obsolete autofocus.
5190cdc
@stopfstedt stopfstedt requested a review from jrjohnson July 8, 2024 22:29
@dartajax dartajax merged commit 3fb4357 into ilios:master Jul 8, 2024
27 checks passed
@stopfstedt stopfstedt mentioned this pull request Jul 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jrjohnson jrjohnson jrjohnson approved these changes

@michaelchadwick michaelchadwick Awaiting requested review from michaelchadwick michaelchadwick is a code owner

Assignees

@dartajax dartajax

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@stopfstedt @michaelchadwick @jrjohnson @dartajax