-
Notifications
You must be signed in to change notification settings - Fork 0
element: ak-tooltip #43
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
base: main
Are you sure you want to change the base?
Conversation
`<ak-tooltip>` implements a tooltip using the native `<dialog>` element as its basis, and exploiting a few tips and tricks from Patternfly’s CSS, so it conforms to the look and feel of the Patternfly React version of the component. In pushing the limits of what is possible, the Tooltip derives stylistic settings from CSS, including the delay for when to show the tooltip! This allows us to configure it so that `prefers-reduced-motion` can be changed to delay the showing and hiding of the tooltip, as well as dicate that the tooltip blink in and out rather than fade. We check the delays on every re-render, so we will pick up changes even in if the user changes the setting in the middle of a session. There is a strong, functionally-oriented state machine that controls the hide/show delay, but there is also a large explanatory comment. The `.wccss` is horribly out-of-date. It was an excellent starting point, but the CSS is highly customized at this point. (I really have to add comments to the WCCSS parser).
* main: (46 commits) Forgot to run , and now that I have a hook to do it for me, this is making it whiny. Fixed typo in css. Added comments to the wcc file. This commit revises the WCCSS transpiler, using the regular expression and merge algorithms recommended by the Patternfly 5 React team. Added wccss pass to package.json. Type inference FTW. Needed to put tools build in-line This commit ports the Elements' build tools to Typescript Cleanup. New css build completing. Crawling back to the original starting point, but this time with better tooling. Save point. This commit introduces a dark-mode controller and code. A working dark mode, although with some dubious decisions Build system revised into a more unixy pattern: each script does one thing well. Bump validator from 13.15.15 to 13.15.20 (#10) Bump vite (#9) First stab at the dark-mode controller; it takes orders from the central context, if there is one, but if not, it tries to determine the mode itself by looking at the user's preferences. Revised brand to meet a... more modern standard. npm trusted publisher ...
…w delay; fixes problem where the wrong method was being called for 'ScheduledHide.onAnchorEnter'
* main: infrastructure: enforce `compat: baseline widely available` to our linting Added browser-compat: baseline, widely available to the rule set for CSS. Removing deprecated eslint plugin. infrastructure: husky & lint-staged Testing. Test. Test. bug: fix poor formatting during the build process
# What
This commit revises some of the tooltip handling code, mostly because it looked *terrible* in dark mode.
1. Remove the `floating-ui` controls of the arrow. Instead, revise the CSS to use the `floating-ui` position names, and map them to the Patternfly Arrow positions.
This lets us use the Patternfly-5 tooltip look and feel in its entirety, without having to try and do the math ourselves for where to place the arrow.
2. Handle some subtle bugs in the CSS parsing that controls the Tooltip.
3. Provide the Patternfly 5 “offset from anchor” as a CSS variable.
- Required a whole new utility library to parse, interpret, and return the pixel size for a variety of CSS. Right now we handle Rem, Em, and Ch. (Well, and Px, of course).
4. Provide the Dark Mode CSS, which, interestingly enough, exists only on `:root`. No changes to `:host` were necessary. (See Note 1!)
5. Begins the revision of the Patternfly 5 globals to support dark mode as we understand it.
6. Updates the test harness to the latest and greatest Chrome
There seems to be a little churn as other components are brought up-to-date with Stylelint’s clean-up pass, which is why `ak-button.css` got a few changes.
# Notes
**Note 1:**
This is a tactical rather than a strategic choice. By not having every dark-mode-aware element checking either the `<html>` tag or listening for its own thematic settings, we “admit” that the default look of the component is its light-mode, and that without the global CSS it will not respond to dark-mode settings. With the global CSS, of course, everything works fine.
It’s also important to note that the two Dark Mode global variables used by this element do not exist at all in light mode. This leads me to think that we need better names for these variables than the ones we picked up from Patternfly 5.
✅ Deploy Preview for authentik-elements-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…ing with spread operator. Fix types, now that offset is a CSS property.
kensternberg-authentik
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.
Left a few mile markers.
| const capabilities = [ | ||
| { | ||
| "browserName": "chrome", // or "firefox", "microsoftedge", "safari" | ||
| "browserVersion": "stable", |
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.
Wdio recommends this if you get "Chromebrowser version not available" warnings, which I did.
| [part="button"] { | ||
| padding-block: var(--button--PaddingTop) var(--button--PaddingBottom); | ||
| padding-inline: var(--button--PaddingLeft) var(--button--PaddingRight); | ||
| padding-block-start: var(--button--PaddingTop); |
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.
Ah. Right. This is after I updated stylelint to not "fix" shorthand declarations. Leave them as-is.
| * @see {@link Tooltip} - The underlying web component | ||
| */ | ||
| export function akTooltip(options: TooltipProps) { | ||
| const { content, htmlFor, trigger, placement, noArrow, ...rest } = options; |
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.
Going forward, I expect to be using this pattern for the builder. This allows users to attach id, class, style, and even Lit-aware event handlers to the element.
| * after the anchor has been triggered. */ | ||
| --tooltip--ShowDelay: var(--ak-v1-c-tooltip--ShowDelay, 100ms); | ||
| --tooltip--HideDelay: var(--ak-v1-c-tooltip--HideDelay, 150ms); | ||
| --tooltip--Offset: var(--ak-v1-c-tooltip--Offset, 0.75rem); |
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.
In the React version, you have to set this in the Javascript. Support is limited; it only understands PX, CH, EM, and REM, but that's more than the Patternfly-Elements or Patternfly-5-React people can say.
|
|
||
| .m-top, | ||
| .m-top-start, | ||
| .m-top-end { |
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.
These names are not the ones supplied by Patternfly; they've been "translated" to match those emitted by Floating-ui. But it is remarkable that all it took was changing terms like "left" and "right" to "start" and "end" to make Patternfly's tooltips 100% compatible with Floating.
Unfortunately, this locks this us into an LTR pattern. If we want to support RTL languages, we'll have to revisit this.
| } | ||
|
|
||
| html[theme="dark"], | ||
| html[data-theme="dark"] { |
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.
As mentioned in Note 1, these are not the targets Patternfly uses for defining its dark mode, but they are ours. Also, the name BackgroundColor-300 is not defined by default in Patternfly; it only exists when dark mode is defined on the global CSS definitions themselves.
# What - Add JSDoc documentation to ak-tooltip. - Place the state machine in a separate file. Exposes just how narrow the state machine is: it’s *one* call! - Updates `.gitignore` to banish AI memory files. All tests passing.
GirlBossRush
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.
An impressive and comprehensive application of web components. Well done 👏
| inset-block: 0; | ||
| inset-inline: 0; | ||
| inset-block-start: 0; | ||
| inset-block-end: 0; | ||
| inset-inline-start: 0; | ||
| inset-inline-end: 0; |
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.
Nit: not sure if it's possible to make style-lint happy here with the originals, or inset: 0. IMO the original was nice and clean
| import type { Tooltip } from "./ak-tooltip.component.js"; | ||
|
|
||
| // Getting a better tooltip required meeting the goal that moving across the anchor quickly | ||
| // shouldn't cause the tooltip to show up immediately, as that would spam a display with a lot of | ||
| // tooltips. It also shouldn't fade out when the pointer transitions from the anchor to the tooltip | ||
| // itself. | ||
| // | ||
| // So when you hover or focus an anchor element, the tooltip is *scheduled to show*, which can be | ||
| // cancelled by leaving the anchor before it becomes visible. Likewise, when the tooltip is visible, | ||
| // the tooltip is *scheduled to hide* when the pointer transitions away, which can be cancelled | ||
| // by the pointer returning to hover or focus either element. | ||
| // | ||
| // This then becomes a state machine: | ||
| // | ||
| // - Hidden -> (mouseover anchor): Scheduled to Show | ||
| // - Scheduled to Show -> [(mouseout anchor): Show canceled: Hidden, otherwise: Show not cancelled: Showing] | ||
| // - Showing -> [Scheduled to hide] | ||
| // - Scheduled to Hide -> [(mouseover anchor or tooltip): Hide canceled: Showing, | ||
| // otherwise: Hide not cancelled: Hidden] | ||
| // | ||
| // The `TooltipEvent` class contains the API needed to support transitions from one state to another | ||
| // inside a `Tooltip`, and the four states inherit from it. It's completely self- contained, there's | ||
| // no "state manager"; a transition results in calls to the Tooltip API to show or hide, but | ||
| // timeouts are dependent on the state being live so they live on the state itself. |
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.
| import type { Tooltip } from "./ak-tooltip.component.js"; | |
| // Getting a better tooltip required meeting the goal that moving across the anchor quickly | |
| // shouldn't cause the tooltip to show up immediately, as that would spam a display with a lot of | |
| // tooltips. It also shouldn't fade out when the pointer transitions from the anchor to the tooltip | |
| // itself. | |
| // | |
| // So when you hover or focus an anchor element, the tooltip is *scheduled to show*, which can be | |
| // cancelled by leaving the anchor before it becomes visible. Likewise, when the tooltip is visible, | |
| // the tooltip is *scheduled to hide* when the pointer transitions away, which can be cancelled | |
| // by the pointer returning to hover or focus either element. | |
| // | |
| // This then becomes a state machine: | |
| // | |
| // - Hidden -> (mouseover anchor): Scheduled to Show | |
| // - Scheduled to Show -> [(mouseout anchor): Show canceled: Hidden, otherwise: Show not cancelled: Showing] | |
| // - Showing -> [Scheduled to hide] | |
| // - Scheduled to Hide -> [(mouseover anchor or tooltip): Hide canceled: Showing, | |
| // otherwise: Hide not cancelled: Hidden] | |
| // | |
| // The `TooltipEvent` class contains the API needed to support transitions from one state to another | |
| // inside a `Tooltip`, and the four states inherit from it. It's completely self- contained, there's | |
| // no "state manager"; a transition results in calls to the Tooltip API to show or hide, but | |
| // timeouts are dependent on the state being live so they live on the state itself. | |
| /** | |
| * @fileoverview This module implements a state machine for tooltip behavior that ensures | |
| * smooth user interactions by preventing tooltip spam when moving across anchor elements | |
| * quickly, while allowing seamless transitions between anchor and tooltip elements. | |
| * | |
| * The `TooltipEvent` class contains the API needed to support transitions from one state to another | |
| * inside a `Tooltip`, and the four states inherit from it. It's completely self- contained, there's | |
| * no "state manager"; a transition results in calls to the Tooltip API to show or hide, but | |
| * timeouts are dependent on the state being live so they live on the state itself. | |
| * | |
| * Getting a better tooltip required meeting the goal that moving across the anchor quickly | |
| * shouldn't cause the tooltip to show up immediately, as that would spam a display with a lot of | |
| * tooltips. It also shouldn't fade out when the pointer transitions from the anchor to the tooltip | |
| * itself. | |
| * | |
| * So when you hover or focus an anchor element, the tooltip is *scheduled to show*, which can be | |
| * cancelled by leaving the anchor before it becomes visible. Likewise, when the tooltip is visible, | |
| * the tooltip is *scheduled to hide* when the pointer transitions away, which can be cancelled | |
| * by the pointer returning to hover or focus either element. | |
| * | |
| * The state machine handles four distinct states: | |
| * - **Hidden**: Initial state, tooltip is not visible | |
| * - **Scheduled to Show**: Tooltip is queued to appear after hover/focus on anchor | |
| * - **Showing**: Tooltip is currently visible to the user | |
| * - **Scheduled to Hide**: Tooltip is queued to disappear after pointer leaves elements | |
| * | |
| * State transitions: | |
| * - Hidden → Scheduled to Show (on anchor mouseover/focus) | |
| * - Scheduled to Show → Hidden (on anchor mouseout before timeout) OR Showing (timeout completes) | |
| * - Showing → Scheduled to Hide (on pointer leave) | |
| * - Scheduled to Hide → Showing (on pointer return) OR Hidden (timeout completes) | |
| */ | |
| import type { Tooltip } from "./ak-tooltip.component.js"; |
| public onTooltipEnter = () => { | ||
| /* no op */ | ||
| }; | ||
|
|
||
| public onTooltipLeave = () => { | ||
| /* no op */ | ||
| }; | ||
|
|
||
| public onAnchorEnter = () => { | ||
| /* no op */ | ||
| }; | ||
|
|
||
| public onAnchorLeave = () => { | ||
| /* no op */ | ||
| }; |
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.
| public onTooltipEnter = () => { | |
| /* no op */ | |
| }; | |
| public onTooltipLeave = () => { | |
| /* no op */ | |
| }; | |
| public onAnchorEnter = () => { | |
| /* no op */ | |
| }; | |
| public onAnchorLeave = () => { | |
| /* no op */ | |
| }; | |
| public onTooltipEnter?: () => void | |
| public onTooltipLeave?: () => void | |
| public onAnchorEnter?: () => void | |
| public onAnchorLeave?: () => void |
| constructor(host: Tooltip) { | ||
| this.host = host; | ||
| } |
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.
| constructor(host: Tooltip) { | |
| this.host = host; | |
| } | |
| constructor(protected host: Tooltip) {} |
|
|
||
| abstract class TooltipEvents { | ||
| protected type: string = ""; | ||
| protected timer: Timeout = null; |
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.
Node's typing on setTimeout is tragic and makes for some interesting work arounds. I want to say this can be avoided on the latest @types/node
Also as much as I love null as initial values, the overlap of the runtime type for Node and the browser is number
| protected timer: Timeout = null; | |
| protected timeoutID = -1; |
| this.dialog.value?.addEventListener("focus", this.#onTooltipEnter, signal); | ||
| this.dialog.value?.addEventListener("blue", this.#onTooltipLeave, signal); | ||
| if (this.trigger === "hover") { | ||
| this.dialog.value?.addEventListener("mouseenter", this.#onTooltipEnter, signal); | ||
| this.dialog.value?.addEventListener("mouseleave", this.#onTooltipLeave, signal); | ||
| } | ||
| } |
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.
| this.dialog.value?.addEventListener("focus", this.#onTooltipEnter, signal); | |
| this.dialog.value?.addEventListener("blue", this.#onTooltipLeave, signal); | |
| if (this.trigger === "hover") { | |
| this.dialog.value?.addEventListener("mouseenter", this.#onTooltipEnter, signal); | |
| this.dialog.value?.addEventListener("mouseleave", this.#onTooltipLeave, signal); | |
| } | |
| } | |
| const dialog = this.dialog.value; | |
| dialog.addEventListener("focus", this.#onTooltipEnter, signal); | |
| dialog.addEventListener("blue", this.#onTooltipLeave, signal); | |
| if (this.trigger === "hover") { | |
| dialog.addEventListener("mouseenter", this.#onTooltipEnter, signal); | |
| dialog.addEventListener("mouseleave", this.#onTooltipLeave, signal); | |
| } | |
| } |
| // window.customElements.define("ak-tooltip", Tooltip); | ||
| // To use the debugging version: | ||
| // ``` | ||
| // import { TooltipWithHover } from "./ak-tooltip.debug.js"; | ||
| // window.customElements.define("ak-tooltip", TooltipWithHover(Tooltip)); | ||
| // ``` | ||
| // |
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.
| // window.customElements.define("ak-tooltip", Tooltip); | |
| // To use the debugging version: | |
| // ``` | |
| // import { TooltipWithHover } from "./ak-tooltip.debug.js"; | |
| // window.customElements.define("ak-tooltip", TooltipWithHover(Tooltip)); | |
| // ``` | |
| // |
| public override render() { | ||
| const fromSlot = this.textContent?.trim() || this.childNodes.length > 0; | ||
| const content = fromSlot ? html`<slot></slot>` : this.content; | ||
| return html`<dialog ${ref(this.dialog)} part="tooltip" role="tooltip" aria-live="polite"> |
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 believe this needs a check if the content is rich i.e. interactive
role="tooltip": Non-interactive, text-only contentrole="dialog": Interactive content, focus management required
| this.dialog.value?.addEventListener("blue", this.#onTooltipLeave, signal); | ||
| if (this.trigger === "hover") { | ||
| this.dialog.value?.addEventListener("mouseenter", this.#onTooltipEnter, signal); | ||
| this.dialog.value?.addEventListener("mouseleave", this.#onTooltipLeave, signal); |
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.
| this.dialog.value?.addEventListener("mouseleave", this.#onTooltipLeave, signal); | |
| this.dialog.value?.addEventListener("mouseleave", this.#onTooltipLeave, | |
| signal); | |
| this.dialog.value?.addEventListener("touchstart", this.#onTooltipEnter, signal); | |
| const signal = { signal: this.#tooltipAbortController.signal }; | ||
| this.dialog.value?.addEventListener("focus", this.#onTooltipEnter, signal); | ||
| this.dialog.value?.addEventListener("blue", this.#onTooltipLeave, signal); | ||
| if (this.trigger === "hover") { |
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.
This should activate on keyboard focus:
A tooltip typically becomes visible, after a short delay of generally one to five seconds, in response to a mouse hover, or after the owning element receives keyboard focus. Just as it is opened automatically, without user request, it is closed automatically when the focus is lost or on mouse out. It must stay open when the mouse moves over the tooltip itself, and should also close when the user presses the Escape key.
What
This commit revises some of the tooltip handling code, mostly because it looked terrible in dark mode.
Remove the
floating-uicontrols of the arrow. Instead, revise the CSS to use thefloating-uiposition names, and map them to the Patternfly Arrow positions.This lets us use the Patternfly-5 tooltip look and feel in its entirety, without having to try and do the math ourselves for where to place the arrow.
Handle some subtle bugs in the CSS parsing that controls the Tooltip.
Provide the Patternfly 5 “offset from anchor” as a CSS variable.
Provide the Dark Mode CSS, which, interestingly enough, exists only on
:root. No changes to:hostwere necessary. (See Note 1!)Begins the revision of the Patternfly 5 globals to support dark mode as we understand it.
Updates the test harness to the latest and greatest Chrome
There seems to be a little churn as other components are brought up-to-date with Stylelint’s clean-up pass, which is why
ak-button.cssgot a few changes.Notes
Note 1:
This is a tactical rather than a strategic choice. By not having every dark-mode-aware element checking either the
<html>tag or listening for its own thematic settings, we “admit” that the default look of the component is its light-mode, and that without the global CSS it will not respond to dark-mode settings. With the global CSS, of course, everything works fine.It’s also important to note that the two Dark Mode global variables used by this element do not exist at all in light mode. This leads me to think that we need better names for these variables than the ones we picked up from Patternfly 5.