-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add tooltip to search #2758
base: main
Are you sure you want to change the base?
Add tooltip to search #2758
Conversation
@nickcharlton still a work in progress but would appreciate any feedback on the approach so far! |
Nice! Thanks for opening this! I think to avoid custom JavaScript for the anchor positioning, we should use a polyfill instead. |
You can add collection filters to a dashboard and then use them in the search bar on the index page. eg. `email:[email protected]` However, these collection filters are not discoverable. You have to know about them already to know how to use them. This change introduces a tooltip which lists the available collection filters, allowing all users to discover what is available. The tooltip icon is `question-mark-circle` from [heroicons] (MIT licensed) We use [anchor positioning] with a [polyfill] to position the [popover] by the tooltip. Some basic JavaScript is introduced to show/hide the popover on hover to ensure the tooltip behaviour works as expected for a tooltip. I've had a first pass at the design, borrowing from existing styles. But I don't feel strongly about it if we want to go in a different direction. Do we want an option to disable the tooltip even if there are collection filters? [heroicons]: https://heroicons.com/solid [anchor positioning]: https://developer.mozilla.org/en-US/docs/Web/CSS/position-anchor [polyfill]: https://github.com/oddbird/css-anchor-positioning [popover]: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/popover Closes #2750
07bca42
to
7b11e28
Compare
@@ -5,3 +5,13 @@ import TableController from "./table_controller"; | |||
|
|||
application.register("select", SelectController); | |||
application.register("table", TableController); | |||
|
|||
// Bug: Visit another page and come back to this page, the mouesover popover will not work |
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.
I'm not sure why this doesn't work after navigating to another page and back. Is it something to do with Turbo page loads?
padding: 0; | ||
|
||
&:hover { | ||
background-color: unset !important; |
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 linter doesn't like !important
here. This whole class is used to effectively unset all the default styles on buttons because a popover needs to use a button from what I understand
left: anchor(center); | ||
margin: 1rem; | ||
position: fixed; | ||
position-anchor: --tooltip-anchor; | ||
top: anchor(bottom); | ||
transform: translateX(-50%); | ||
|
||
background-color: $blue; | ||
border-color: $blue; | ||
border-radius: $base-border-radius; | ||
color: $white; | ||
padding: 2rem; | ||
width: max-content; |
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 first set of styles is all to do with positioning the popover, the second set (after the gap) is all to do with the visual style of the popover. The linter doesn't like the gap and the non alphabetical between them. It feels useful to separate the styles as I have. Shall I created 2 separate classes?
TODO: Move where this polyfill lives. Where? | ||
Also, do we want to import the polyfill js directly instead of linking to unpkg.com? | ||
%> | ||
<script type="module"> | ||
if (!("anchorName" in document.documentElement.style)) { | ||
import("https://unpkg.com/@oddbird/css-anchor-positioning"); | ||
} |
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.
- Where should this polyfill live?
- Do we want to import the polyfill js directly instead of linking to unpkg.com?
|
||
render locals: { | ||
resources: resources, | ||
search_term: search_term, | ||
page: page, | ||
show_search_bar: show_search_bar? | ||
show_search_bar: show_search_bar?, | ||
filters: filters |
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.
Do we want an option to disable the tooltip even if there are collection
filters?
@nickcharlton I think this is in a decent state for a review now. I've updated the code, description and added some new screenshots. I've left some comments where I could do with some guidance. There is also a test failing which I'm not sure about. |
You can add collection filters to a dashboard and then use them in the
search bar on the index page. eg.
email:[email protected]
However, these collection filters are not discoverable. You have to know
about them already to know how to use them.
This change introduces a tooltip which lists the available collection
filters, allowing all users to discover what is available.
The tooltip icon is
question-mark-circle
from heroicons (MIT licensed)We use anchor positioning with a polyfill to position the popover by
the tooltip.
Some basic JavaScript is introduced to show/hide the popover on hover to
ensure the tooltip behaviour works as expected for a tooltip.
I've had a first pass at the design, borrowing from existing styles. But
I don't feel strongly about it if we want to go in a different direction.
Do we want an option to disable the tooltip even if there are collection
filters?
Closes #2750