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

new(Overlay): Add enableMouseInteraction prop #391

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

espressoroaster
Copy link

@espressoroaster espressoroaster commented Jul 23, 2020

to: @williaster @alecklandgraf

Description

This PR adds the enableMouseInteraction prop to Overlay and Overlay/Portal to enable mouse events and mouse interaction in Overlays. In the Overlay portal stylesheet, we extract the container class' userSelect: 'none' out into a noUserSelect class, and introduce the hasPointerEvents withPointerEvents class to explicitly enable pointer events in Overlay. These two classes are composed in such a way that there are no breaking changes.

Motivation and Context

Overlay's portal implementation is useful for Tooltip and Popover popups, which need to escape their parent divs. The current Popover implementation renders the popup in a div with a set zIndex, which does not let the popup escape a DataTable row, while a plain Tooltip's <Overlay></Overlay> popup is able to escape the DataTable as intended.

To allow Popover to use <Overlay> for its Portal rendering and positioning logic, we added the ability to enable pointer events in Overlay so that Popover's mouse event based timeout logic and copy-paste functionality could work when implemented with Overlay.

Testing

Tested in storybook.

Screenshots

An overlay without enableMouseInteraction prop:
overlay-no-prop

An overlay with enableMouseInteraction prop (mouse can interact with content in the overlay):
overlay-with-prop

Checklist

  • My code follows the style guide of this project.
  • I have updated or added documentation accordingly.
  • I have read the CONTRIBUTING document.

@airbnb-bot
Copy link
Collaborator

airbnb-bot commented Jul 23, 2020

Size Changes

Package Diff ESM Prev ESM CJS Prev CJS
core +0.1% 566.94 KB 566.56 KB 709.02 KB 708.61 KB

Compared to master. File sizes are unminified and ungzipped.

View raw build stats

Previous (master)

{
  "apollo": {
    "esm": 10832,
    "lib": 14147
  },
  "app-shell": {
    "esm": 12906,
    "lib": 19874
  },
  "composer": {
    "esm": 68247,
    "lib": 101805
  },
  "core": {
    "esm": 580157,
    "lib": 725612
  },
  "forms": {
    "esm": 37350,
    "lib": 49298
  },
  "icons": {
    "esm": 156355,
    "lib": 205626
  },
  "layouts": {
    "esm": 15298,
    "lib": 20770
  },
  "metrics": {
    "esm": 5467,
    "lib": 7729
  },
  "test-utils": {
    "esm": 4279,
    "lib": 5937
  }
}

Current

{
  "apollo": {
    "esm": 10832,
    "lib": 14147
  },
  "app-shell": {
    "esm": 12906,
    "lib": 19874
  },
  "composer": {
    "esm": 68247,
    "lib": 101805
  },
  "core": {
    "esm": 580551,
    "lib": 726036
  },
  "forms": {
    "esm": 37350,
    "lib": 49298
  },
  "icons": {
    "esm": 156355,
    "lib": 205626
  },
  "layouts": {
    "esm": 15298,
    "lib": 20770
  },
  "metrics": {
    "esm": 5467,
    "lib": 7729
  },
  "test-utils": {
    "esm": 4279,
    "lib": 5937
  }
}

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

Nice! lgtm. trying to re-run CI tests

userSelect: 'none',
},

hasPointerEvents: {
Copy link
Contributor

Choose a reason for hiding this comment

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

wdyt about withPointerEvents instead?

Copy link
Author

Choose a reason for hiding this comment

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

sgtm, will make that change and push

@espressoroaster espressoroaster merged commit 26e905d into master Jul 23, 2020
@espressoroaster espressoroaster deleted the fo--refactor-overlay branch July 23, 2020 21:26
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.

3 participants