Skip to content

Conversation

Westbrook
Copy link
Contributor

@Westbrook Westbrook commented May 24, 2021

Description

Add a virtualTrigger option to the overlay process that will supersede the element trigger, when available, as part of interaction directly with PopperJS.

new VirtualTrigger(x, y) will now provide a VirtualElement that can be passed an option to the openOverlay methods for use as the "trigger" in calls to PopperJS. The element makes public both a getBoundingClientRect() method for position the overlay and a updateBoundingClientRect(x, y) method for moving the "trigger" of the overlay. updateBoundingClientRect() also calls Overlay.update() internally to reduce the amount of manual interaction required to update overlays opened with the virtual trigger context.

Related Issue

fixes #1437

Motivation and Context

Attempt to simplify from #1445

How Has This Been Tested?

Manually in Storybook. If this approach makes sense, some unit tests will be added.

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.
  • 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.

Copy link
Collaborator

@najikahalsema najikahalsema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me so far!

@Westbrook Westbrook force-pushed the westbrook/overlay-triggers branch 2 times, most recently from e79a6c2 to 016e597 Compare May 28, 2021 11:51
@Westbrook Westbrook force-pushed the westbrook/overlay-triggers branch from 016e597 to 11e0459 Compare May 28, 2021 21:24
adixon-adobe
adixon-adobe previously approved these changes Jun 1, 2021
Copy link
Collaborator

@adixon-adobe adixon-adobe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a much better approach. Nice!

openOverlay(
trigger,
'modal',
trigger.nextElementSibling as HTMLElement,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding this right, clients still pass in an HTMLElement, but now that's just for event bubbling (and focus?), but the virtualTrigger manages positioning. Seems good to me!

@Westbrook Westbrook marked this pull request as ready for review June 1, 2021 14:43
@Westbrook Westbrook changed the title [RFC] Virtual Trigger as additional option to split what's send to Popper and SWC Virtual Trigger as additional option to split what's send to Popper and SWC Jun 1, 2021
@Westbrook Westbrook merged commit 34d914d into main Jun 2, 2021
@Westbrook Westbrook deleted the westbrook/overlay-triggers branch June 2, 2021 11:24
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.

[Overlay] Add Virtual Element support for triggering overlays

4 participants