Skip to content

Conversation

jeferson-sb
Copy link
Contributor

What does this PR do?

event.path has been deprecated and no longer works in firefox,
composedPath() method returns path of listeners of that click event which is being used to determine whether the click occur outside of the Layer portal or not.

Where should the reviewer start?

src/js/components/Layer/LayerContainer.js

What testing has been done on this PR?

yarn test

How should this be manually tested?

Test storybooks like this one and
ensure clicking outside of either the first or second modal closes it.

Do Jest tests follow these best practices?

  • screen is used for querying.
  • The correct query is used. (Refer to this list of queries)
  • userEvent is used in place of fireEvent.
  • asFragment() is used for snapshot testing.

Any background context you want to provide?

No

What are the relevant issues?

See #6452

Screenshots (if appropriate)

Do the grommet docs need to be updated?

No

Should this PR be mentioned in the release notes?

Is this change backwards compatible or is it a breaking change?

backwards compatible

Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com>
jcfilben
jcfilben previously approved these changes Oct 28, 2022
Copy link
Collaborator

@jcfilben jcfilben 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!

@jcfilben jcfilben dismissed their stale review October 28, 2022 15:30

Realized we need [0] at the end of event?.composedPath()

jeferson-sb and others added 2 commits October 28, 2022 18:32
Co-authored-by: Jessica Filben <54560994+jcfilben@users.noreply.github.com>
@jeferson-sb jeferson-sb requested a review from jcfilben October 29, 2022 17:45
Copy link
Collaborator

@jcfilben jcfilben 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!

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