Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(Popup): click/keypress handling logic for content #1482

Merged
merged 5 commits into from
Jun 11, 2019

Conversation

kuzhelov
Copy link
Contributor

@kuzhelov kuzhelov commented Jun 10, 2019

Fixes #1324

TODO

  • update changelog

Problem

Click on popup content's button toggles popup content to close.

Cause

The cause of the problem is rather convoluted. The reason popup closes is because it 'sees' click on the content's button as outside click.

Why? Popup has the following logic to detect outside clicks

  • collect refs to all inner elements (incl potential refs to inner nested popups)
  • on click check if click is made outside any of these refs' areas
    • if it is an outside click, close the popup

While idea is straightforward, it turns out that currently we have problems caused by order of click events processing. When button inside of popup content is clicked,

  • DOM tree updates
  • and only after the logic of checking refs' areas is taking place. At that moment, however, click's coordinates doesn't match any refs' updated areas.

Resolution

To address it, for now, capture prop of popup's <EventListener /> is used - this activates capture subscription mode for DOM events, which guarantees that corresponding event handler will be invoked before any other event's effects. (https://developer.mozilla.org/ru/docs/Web/API/EventTarget/addEventListener)

Additional notes

These changes will be merged after #1463 , where e2e (browser) test is introduced for the issue's case.

@kuzhelov kuzhelov changed the title fix(Popup): click handling logic for content fix(Popup): click/keypress handling logic for content Jun 10, 2019
@codecov
Copy link

codecov bot commented Jun 10, 2019

Codecov Report

Merging #1482 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1482   +/-   ##
=======================================
  Coverage   73.59%   73.59%           
=======================================
  Files         807      807           
  Lines        6117     6117           
  Branches     1804     1785   -19     
=======================================
  Hits         4502     4502           
  Misses       1609     1609           
  Partials        6        6
Impacted Files Coverage Δ
packages/react/src/components/Popup/Popup.tsx 67.74% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fae74b5...c880286. Read the comment docs.

@kuzhelov kuzhelov merged commit 74f1a5a into master Jun 11, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/popup-content-click-handling branch June 11, 2019 09:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Popup is closed incorrectly when click inside element
3 participants