Skip to content
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

fix(overlay): add overlay lifecycle methods to stack management #1393

Merged
merged 1 commit into from
May 19, 2021

Conversation

Westbrook
Copy link
Contributor

@Westbrook Westbrook commented Apr 20, 2021

Description

  • create "Overlay Lifecycle" methods that call into the overlaid element at open and close time:
    • overlayWillOpenCallback marks the beginning of the open cycle. by default this is followed in a few short frames with the actual opening of the overlay and in this way can be seen as equivalent to an overlayOpenedCallback for most concerns)
    • overlayOpenCancelledCallback takes into account overlay content that is delayed (usually tooltips) and could have their open process cancelled.
    • overlayCloseCallback handles when the entire cycle is complete and only can occur for overlays that had not previously called overlayOpenCancelledCallback.
    • each method accept an options argument with { trigger: HTMLElement } (to support expanding to more options as needed in the future) and returns void
  • leverage these lifecycle methods in the sp-tooltip element to appropriate hang content in the origin location to be used for accessibility while the actual sp-tooltip element is in "overlay" position and not accessible via IDREFs.
    • add hidden content with the sp-tooltip's textContent and appropriate aria attributes across the overlay experience to ensure that the trigger is "described by" the tooltip.
  • overlay-trigger now handles an abortOverlay promise for hover-content so that in the likelihood that said content is delayed the warmup time can be cancelled appropriately.
  • add testing for the above.

Alternative

Right now we dispatch sp-opened and sp-closed events on the trigger element of the overlay feature. We would be similarly served as the above were we to dispatch the same events on the content elements as well and leverage event listeners rather than direct method calls. We could place bubbles: false on one of those dispatches to prevent bubble phases on both events, but we would still have capture phases on both which means that elements that might listen to these events above might face challenges disambiguating one event from the other. In this way we could dispatch more specific events sp-opened and sp-content-opened or more specific detail packages:

{
    interaction: TriggerInteractions,
    type: 'trigger' | 'content',
    trigger: HTMLElement,
    content: HTMLElement,
}

vs:

{
    interaction: TriggerInteractions,
}

However, there is something to be said about the pseudo "privacy" of this approach, as well as its relation to the custom element and LitElement usage of lifecycle methods.

Related Issue

fixes #1078

Motivation and Context

More accessible tooltips.
More capable overlay system.
World peace?

How Has This Been Tested?

  • manual with both the chrome dev tools accessibility tree and macOS Voiceover x-brower
  • unit tests leveraging the accessibility tree across chromium, webkit and firefox.

Screenshots (if appropriate):

image
image

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
    • Maybe. But, I think we should keep this sort of API pseudo "private" for a little to shake out the wrinkled before documenting.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@Westbrook Westbrook force-pushed the westbrook/tooltip branch 3 times, most recently from 55fa970 to 23b8a90 Compare April 22, 2021 18:41
@Westbrook Westbrook changed the title [RFC] fix(overlay): add overlay lifecycle methods to stack management fix(overlay): add overlay lifecycle methods to stack management Apr 26, 2021
@Westbrook Westbrook marked this pull request as ready for review April 26, 2021 13:50
@Westbrook Westbrook force-pushed the westbrook/tooltip branch 6 times, most recently from 78a2fb0 to aee0137 Compare May 2, 2021 18:05
@Westbrook Westbrook force-pushed the westbrook/tooltip branch 2 times, most recently from e2a0dbf to 59bf11c Compare May 5, 2021 16:47
@Westbrook Westbrook marked this pull request as draft May 6, 2021 12:31
@Westbrook Westbrook force-pushed the westbrook/tooltip branch 2 times, most recently from 453ded7 to 98b868f Compare May 6, 2021 17:52
@Westbrook
Copy link
Contributor Author

I've updated the lifecycle naming based off of some feedback from @jstoeckm both in the code and the description above.

I've also updated the overlayOpenCallback to be overlayWillOpenCallback and overlayOpenCancelledCallback to defeat the screen reader timing outline in #1078 (comment)

@Westbrook Westbrook marked this pull request as ready for review May 6, 2021 18:00
@Westbrook Westbrook force-pushed the westbrook/tooltip branch 5 times, most recently from 4fb1900 to d483ba4 Compare May 12, 2021 14:55
@Westbrook Westbrook force-pushed the westbrook/tooltip branch from d483ba4 to 384e3a8 Compare May 18, 2021 19:52
@Westbrook Westbrook merged commit 9361527 into main May 19, 2021
@Westbrook Westbrook deleted the westbrook/tooltip branch May 19, 2021 19:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tooltip accessibility review
2 participants