-
Notifications
You must be signed in to change notification settings - Fork 254
Redesign facet suggest input styling/layout #3769
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
Conversation
|
Personally I don't think it needs/wants placeholder text at all, but the i18n key was already there, and can be easily overriden locally to empty string I suppose. Or happy to remove it (and the i18n key(s)) if consensus is it's not needed. @sandbergja , interested in reviewing? |
Note that the previous one had a typo/mistake in i18n label -- there was already an i18n key entered for this, but it was using a different one, now it's using the intended one.
00b4b65 to
278e713
Compare
sandbergja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, @jrochkind -- the more compact layout is nice
| %> | ||
| <div class="input-group"> | ||
| <label class="input-group-text" for="facet_suggest_<%= key %>"> | ||
| <%= I18n.t('blacklight.search.facets.suggest.label', field_label: label.downcase.pluralize) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍🏻 -- the .downcase.pluralize is a nice refinement
|
Not sure if the little spyglass on the end needs an aria label... or should just be omitted, maybe it's too fussy and unnecessary? Any opinions? |
It seems like a decorative image to me, I would lean toward adding |
|
Thanks! i guess blacklight already puts aria-hidden on the SVG, so I guess it's fine.
|
Note that the previous one had a typo/mistake in i18n label -- there was already an i18n key entered for this, but it was using a different one, now it's using the intended one.
Before
After
input is type=search
So on some browsers (Chrome and Safari, but not Firefox), you get a "erase input"
xicon, which works very nicely with the JS to immediately reset to unfiltered display.