Stimulus for search field autocomplete #4468
Conversation
martinemde
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4468 +/- ##
=======================================
Coverage 97.15% 97.15%
=======================================
Files 391 391
Lines 8260 8261 +1
=======================================
+ Hits 8025 8026 +1
Misses 235 235 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
should this be a partial instead of a layout? or a phlex component maybe?
There was a problem hiding this comment.
When you say "partial instead of layout" I'm confused. It's a partial in the layouts dir. should it move somewhere else?
Phlex components seem cool but they're also an extra barrier to entry. This is mostly code though, so it is reasonable. I thought about just making it a helper.
I'd be really happy to watch you run through the component stuff you made so I could start using it.
There was a problem hiding this comment.
Phlex components seem cool but they're also an extra barrier to entry. This is mostly code though, so it is reasonable. I thought about just making it a helper.
👍 I agree on this. Should we open discussion somewhere to get in sync on this topic? Meanwhile let's not block template changes because of this if possible.
martinemde
force-pushed
the
martinemde/stimulus-autocomplete
branch
from
78ddb8a to
c53f139
Compare
| @@ -0,0 +1,98 @@ | |||
| import { Controller } from "@hotwired/stimulus" | |||
|
|
|||
| // TODO: Add suggest help text and aria-live | |||
There was a problem hiding this comment.
Is this TODO for later (other PR)?
There was a problem hiding this comment.
Yeah, it's kind of a burden to add this here and requires some extra style and i18n work.
martinemde
force-pushed
the
martinemde/stimulus-autocomplete
branch
from
c53f139 to
8f49d59
Compare
martinemde
force-pushed
the
martinemde/nav-while-logged-out
branch
from
16ea06d to
e1dec5f
Compare
martinemde
force-pushed
the
martinemde/stimulus-autocomplete
branch
2 times, most recently
from
af1ef5c to
6319cb0
Compare
martinemde
force-pushed
the
martinemde/stimulus-autocomplete
branch
from
6319cb0 to
3fb86fb
Compare
martinemde
force-pushed
the
martinemde/stimulus-autocomplete
branch
from
3fb86fb to
14dcd56
Compare
|
Responded to feedback and rebased. |
Mostly identical in behavior, but removes jQuery and unifies the search form html besides a few classes. Adds little improvements to accessibility and a bit nicer handling of other common events like clicking out or hitting escape.
This also changes the way the search is loaded into the header, skipping rendering in pages that already have a search field in-page. This means there's always only one search field on a page.