Improve compat notes and behavior on popover-hint demo #329
Conversation
chrisdavidmills
requested review from
dipikabh and
pepelsbey
and removed request for
a team
pepelsbey
left a comment
pepelsbey
left a comment
There was a problem hiding this comment.
Sorry it took some time! A few suggestions for you to consider :)
popover-api/popover-hint/index.css
Outdated
| margin: 0; | ||
| padding: 16px; | ||
| box-shadow: 1px 1px 3px #999; | ||
| z-index: -1; |
There was a problem hiding this comment.
This line makes the paragraph unreachable to the cursor, as it’s hidden behind the body: no text selection, no link clicking.
There was a problem hiding this comment.
Ah, crap, that was foolish! In my next commit I have adjusted the z-index values to fix this issue.
popover-api/popover-hint/index.html
Outdated
| <div id="tooltip-1" class="tooltip" popover="hint">Tooltip A</div> | ||
| <div id="tooltip-2" class="tooltip" popover="hint">Tooltip B</div> | ||
| <div id="tooltip-3" class="tooltip" popover="hint">Tooltip C</div> |
There was a problem hiding this comment.
I would suggest putting tooltips right next to the buttons they’re triggered by. First of all, this is something you would most likely do in a real-world component to contain everything. Second, it might help to position them next to the buttons as a fallback for browsers that don’t support the hint value yet:
<button
popovertarget="submenu-1"
popovertargetaction="toggle"
id="menu-1">
Menu A
</button>
<div id="tooltip-1" class="tooltip" popover="hint">
Tooltip A
</div>There was a problem hiding this comment.
I've done that, experimented with moving the HTML around a bit more besides, and made a few CSS adjustments, but I still can't make it work particularly well in browsers that don't support anchor positioning. Do you have any suggestions?
popover-api/popover-hint/index.html
Outdated
| <button | ||
| popovertarget="submenu-1" | ||
| popovertargetaction="toggle" | ||
| id="menu-1"> | ||
| Menu A | ||
| </button> | ||
|
|
||
| <div id="tooltip-1" class="tooltip" popover="hint">Tooltip A</div> | ||
|
|
||
| <button | ||
| popovertarget="submenu-2" | ||
| popovertargetaction="toggle" | ||
| id="menu-2"> | ||
| Menu B | ||
| </button> | ||
|
|
||
| <div id="tooltip-2" class="tooltip" popover="hint">Tooltip B</div> | ||
|
|
||
| <button | ||
| popovertarget="submenu-3" | ||
| popovertargetaction="toggle" | ||
| id="menu-3"> | ||
| Menu C | ||
| </button> | ||
| </section> | ||
| </div> | ||
|
|
||
| <p class="help-para"> | ||
| Press the buttons to show the auto popovers. Hover or focus the buttons to | ||
| show the hint popovers. When an auto popover is shown, you can show the | ||
| hint popovers without hiding it. See | ||
| <a | ||
| href="https://developer.mozilla.org/en-US/docs/Web/API/Popover_API/Using#using_hint_popover_state" | ||
| >Using "hint" popover state</a | ||
| > | ||
| for more information. Note that in non-supporting browsers, the position | ||
| of the popovers is broken. | ||
| </p> | ||
| <div id="tooltip-3" class="tooltip" popover="hint">Tooltip C</div> |
There was a problem hiding this comment.
If you put the tooltips inside buttons, it falls back nicely!
| <div id="tooltip-3" class="tooltip" popover="hint">Tooltip C</div> | |
| <button | |
| popovertarget="submenu-1" | |
| popovertargetaction="toggle" | |
| id="menu-1"> | |
| Menu A | |
| <div id="tooltip-1" class="tooltip" popover="hint">Tooltip A</div> | |
| </button> | |
| <button | |
| popovertarget="submenu-2" | |
| popovertargetaction="toggle" | |
| id="menu-2"> | |
| Menu B | |
| <div id="tooltip-2" class="tooltip" popover="hint">Tooltip B</div> | |
| </button> | |
| <button | |
| popovertarget="submenu-3" | |
| popovertargetaction="toggle" | |
| id="menu-3"> | |
| Menu C | |
| <div id="tooltip-3" class="tooltip" popover="hint">Tooltip C</div> | |
| </button> |
There was a problem hiding this comment.
If you put the tooltips inside buttons, it falls back nicely!
Ah yes, the tooltips certainly sit more nicely. The menus are still a bit broken, but it is better than before.
2 participants
As discussed in mdn/content#40881, the browser support situation in our
popover="hint"demo is not very clear. In browsers that don't supportpopover="hint", the tooltip behavior is not quite right. And in browsers that don't support CSS anchor positioning, the demo looks broken because the popovers don't appear as expected.To mitigate these issues, I have updated the demo. Specifically, I have:
#wrapper<div>and given it apositionofrelativeso that they should be positioned relative to the<div>rather than the<body>in non-supporting browsers (note: this still doesan't quite work property. Setting a fallback inset property still seems to position them relative to the<body>, and I'm not sure why)I'm still not 100% happy with this, but the available information is a lot more useful now, and the demo looks less broken in non-supporitng browsers.
Once we've got this update merged, I'll then look at the associated MDN content and see if any of it needs updating as a result.