Skip to content

Fix bug not allowing users to close by clicking on backdrop#313

Closed
guyathomas wants to merge 2 commits intojossmac:masterfrom
guyathomas:fix-unable-to-close-backdrop-on-click
Closed

Fix bug not allowing users to close by clicking on backdrop#313
guyathomas wants to merge 2 commits intojossmac:masterfrom
guyathomas:fix-unable-to-close-backdrop-on-click

Conversation

@guyathomas
Copy link
Copy Markdown
Contributor

Description of changes:
It seems !event.target.classList.contains(className('view') had the intention of only allowing the closing of the backdrop on clicking in a selected region (picture below). However, this area did not actually take up the entire backdrop. Instead of whitelisting allowed areas, i decided to blacklist areas that a click should not close ( previous and next buttons )

image

Related issues (if any):
#302

Checks:

  • Please confirm yarn run lint ran successfully
  • Please confirm that only /src and /examples/src are committed
  • [if new feature] Please confirm that documentation was added to the README.md

@guyathomas guyathomas force-pushed the fix-unable-to-close-backdrop-on-click branch 2 times, most recently from e1adfa4 to f96c60c Compare August 30, 2019 04:30

prev = () => {
prev = (event) => {
event.stopPropagation();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this is needed for the close and fullscreen button as well

@davwheat davwheat self-assigned this Nov 28, 2019
@davwheat davwheat added the bug label Nov 28, 2019
@kyriiherman
Copy link
Copy Markdown

hey, any updates on this PR? 🤔

@davwheat
Copy link
Copy Markdown
Collaborator

I'll need to test the changes and fix the linting error (unless the PR author does it), then it can be merged!

@davwheat
Copy link
Copy Markdown
Collaborator

Fixed by #357

@davwheat davwheat closed this Mar 21, 2020
@davwheat davwheat reopened this May 2, 2020
@davwheat davwheat closed this May 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants