fix(Popup): proper handling of clicks in nested Popups #949

layershifter · 2019-02-22T13:06:05Z

Remove existing `EventStack`

Was completely replaced by @stardust-ui/react-component-event-listener & @stardust-ui/react-component-nesting-registry.

levithomason · 2019-03-06T19:45:11Z

Per our chat, consider a bit of renaming for clarity and alignment with browser APIs:

    <EventHandler
      type='mousedown'
      listener={() => {}}
      targetRef={}
    />

codecov · 2019-03-13T15:37:28Z

Codecov Report

Merging #949 into master will decrease coverage by 0.2%.
The diff coverage is 95.08%.

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
- Coverage    82.3%   82.09%   -0.21%     
==========================================
  Files         715      711       -4     
  Lines        8656     8528     -128     
  Branches     1178     1224      +46     
==========================================
- Hits         7124     7001     -123     
+ Misses       1516     1511       -5     
  Partials       16       16

Impacted Files	Coverage Δ
packages/react/src/lib/index.ts	`100% <ø> (ø)`	⬆️
.../src/lib/accessibility/FocusZone/FocusTrapZone.tsx	`78.35% <100%> (-0.22%)`	⬇️
packages/react/src/components/Portal/Portal.tsx	`98.07% <100%> (-0.08%)`	⬇️
packages/react/src/components/Menu/MenuItem.tsx	`61.26% <50%> (-1.86%)`	⬇️
packages/react/src/components/Popup/Popup.tsx	`76.08% <95.74%> (-0.18%)`	⬇️
packages/react/src/lib/whatInput.ts	`63.38% <0%> (-1.41%)`	⬇️

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 9b969d0...78f2c7e. Read the comment docs.

DustyTheBot · 2019-03-15T13:18:26Z

	Warnings
⚠️	Package (or peer) dependencies changed. Make sure you have approval before merging!

Changed dependencies in packages/react/package.json

package	before	after
@stardust-ui/react-component-event-listener	-	^0.23.1
@stardust-ui/react-component-nesting-registry	-	^0.23.1

Generated by 🚫 dangerJS

miroslavstastny · 2019-03-18T12:59:30Z

packages/react-component-nesting-registry/package.json

+  "files": [
+    "dist"
+  ],
+  "homepage": "https://github.com/stardust-ui/react-component-nesting-registry#readme",


👍

Updated to https://github.com/stardust-ui/react/tree/master/packages/react-component-nesting-registry, it is still 404, but will work after PR will be merged

miroslavstastny · 2019-03-18T14:14:20Z

packages/react/src/components/Popup/Popup.tsx

@@ -470,7 +436,6 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
    const popupWrapperAttributes = {
      ...(rtl && { dir: 'rtl' }),
      ...accessibility.attributes.popup,
-      ...accessibility.keyHandlers.popup,


I think accessibility.keyHandlers were originally planned to be generic enough to contain anything. With this being replaced by keyHandler on document is this still true?
popup-test contains neither isConformant nor handlesAccessibility :-/

Actually, I want to revert this change and rework behavior part separately. What do you think?

…thub.com/stardust-ui/react into feat/children-nesting-api

packages/react-component-nesting-registry/src/lib/RegistrySet.ts

docs/src/examples/components/Popup/Usage/PopupExampleNested.shorthand.tsx

…thub.com/stardust-ui/react into feat/children-nesting-api # Conflicts: # CHANGELOG.md # packages/react-component-nesting-registry/src/NestingRoot.tsx # packages/react-component-nesting-registry/src/hooks/useNestingRoot.ts

layershifter · 2019-03-20T09:25:03Z

FYI: @miroslavstastny @mnajdova I moved keyboard/accessibility issues to a separate issue #1079

layershifter · 2019-03-20T09:42:01Z

packages/react/src/components/Popup/Popup.tsx

+    const popupContent = Popup.Content.create(content, {
+      defaultProps: popupContentAttributes,
+      overrideProps: this.getContentProps,
+    })


We don't need this check more, it was introduced in #592. Now it will be done by factories.

packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md

packages/react/src/components/Popup/Popup.tsx

…thub.com/stardust-ui/react into feat/children-nesting-api # Conflicts: # CHANGELOG.md # packages/react/package.json # packages/react/src/lib/accessibility/FocusZone/FocusTrapZone.tsx

layershifter requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners February 22, 2019 13:06

This was referenced Feb 28, 2019

fix(Popup): close previous popup on enter key #985

Merged

RFC: Ref utils #998

Closed

layershifter force-pushed the feat/children-nesting-api branch 2 times, most recently from d8c82e3 to bf15a47 Compare March 12, 2019 15:04

This was referenced Mar 12, 2019

chore(package): update Enzyme #1051

Merged

feat(events): add EventListener and StackableEventListener components #1052

Merged

layershifter force-pushed the feat/children-nesting-api branch 3 times, most recently from 392ebe7 to 25c5da6 Compare March 13, 2019 12:20

layershifter mentioned this pull request Mar 15, 2019

feat(EventListener): add capture prop #1062

Merged

layershifter force-pushed the feat/children-nesting-api branch 2 times, most recently from e463233 to 7cb4a24 Compare March 15, 2019 13:06

layershifter force-pushed the feat/children-nesting-api branch from 7cb4a24 to 5601fe6 Compare March 15, 2019 14:18

fix(Popup): fix work with nested elements

4c60a83

layershifter force-pushed the feat/children-nesting-api branch from 5601fe6 to 4c60a83 Compare March 15, 2019 14:33

add CHANGELOG entry

6d95b88

layershifter changed the title ~~WIP: new components for Event handling and controlling nesting~~ fix(Popup): proper handling of nested Popups Mar 15, 2019

layershifter added 2 commits March 15, 2019 17:01

move tests

4d7245f

fix UTs

7d0f016

miroslavstastny reviewed Mar 18, 2019

View reviewed changes

layershifter added 🚀 ready for review 🧰 fix Introduces fix for broken behavior. labels Mar 19, 2019

layershifter added 3 commits March 19, 2019 11:56

Merge branches 'feat/children-nesting-api' and 'master' of https://gi…

649237c

…thub.com/stardust-ui/react into feat/children-nesting-api

update versions and fix homepage fields

62b53a6

add Jest config

4e64eb8

layershifter commented Mar 19, 2019

View reviewed changes

packages/react-component-nesting-registry/src/lib/RegistrySet.ts Outdated Show resolved Hide resolved

layershifter commented Mar 19, 2019

View reviewed changes

docs/src/examples/components/Popup/Usage/PopupExampleNested.shorthand.tsx Outdated Show resolved Hide resolved

layershifter mentioned this pull request Mar 19, 2019

feat(Nesting): add NestingAuto component #1075

Merged

layershifter added 5 commits March 19, 2019 18:13

remove cruft file

24de1c7

rename example

ea17098

revert accessibility changes in Popup

9339b71

update CHANGELOG entry

bd79c66

layershifter changed the title ~~fix(Popup): proper handling of nested Popups~~ fix(Popup): proper handling of clicks in nested Popups Mar 20, 2019

layershifter added 2 commits March 20, 2019 11:27

restore changes in Popup tests

4da04c5

add entry to another changelog

07d39a9

layershifter commented Mar 20, 2019

View reviewed changes

miroslavstastny approved these changes Mar 20, 2019

View reviewed changes

packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md Outdated Show resolved Hide resolved

update changelog

2a8eb3f

kuzhelov reviewed Mar 21, 2019

View reviewed changes

packages/react/src/components/Popup/Popup.tsx Outdated Show resolved Hide resolved

layershifter added 2 commits March 21, 2019 15:06

removed all visibility modifiers

664c134

Merge branches 'feat/children-nesting-api' and 'master' of https://gi…

78f2c7e

…thub.com/stardust-ui/react into feat/children-nesting-api # Conflicts: # CHANGELOG.md # packages/react/package.json # packages/react/src/lib/accessibility/FocusZone/FocusTrapZone.tsx

sophieH29 approved these changes Mar 25, 2019

View reviewed changes

layershifter merged commit 8b8bc3d into master Mar 25, 2019

delete-merged-branch bot deleted the feat/children-nesting-api branch March 25, 2019 12:00

layershifter mentioned this pull request Apr 18, 2019

RFC: how should we structure our monorepo and packages #433

Closed

10 tasks

layershifter mentioned this pull request Jul 23, 2019

Modal: keydown event propagates to all modals Semantic-Org/Semantic-UI-React#3716

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Popup): proper handling of clicks in nested Popups #949

fix(Popup): proper handling of clicks in nested Popups #949

layershifter commented Feb 22, 2019 •

edited

levithomason commented Mar 6, 2019

codecov bot commented Mar 13, 2019 •

edited

DustyTheBot commented Mar 15, 2019 •

edited

miroslavstastny Mar 18, 2019

layershifter Mar 19, 2019

miroslavstastny Mar 18, 2019

layershifter Mar 19, 2019

layershifter commented Mar 20, 2019

layershifter Mar 20, 2019 •

edited

fix(Popup): proper handling of clicks in nested Popups #949

fix(Popup): proper handling of clicks in nested Popups #949

Conversation

layershifter commented Feb 22, 2019 • edited

Remove existing EventStack

levithomason commented Mar 6, 2019

codecov bot commented Mar 13, 2019 • edited

Codecov Report

DustyTheBot commented Mar 15, 2019 • edited

miroslavstastny Mar 18, 2019

Choose a reason for hiding this comment

layershifter Mar 19, 2019

Choose a reason for hiding this comment

miroslavstastny Mar 18, 2019

Choose a reason for hiding this comment

layershifter Mar 19, 2019

Choose a reason for hiding this comment

layershifter commented Mar 20, 2019

layershifter Mar 20, 2019 • edited

Choose a reason for hiding this comment

layershifter commented Feb 22, 2019 •

edited

Remove existing `EventStack`

codecov bot commented Mar 13, 2019 •

edited

DustyTheBot commented Mar 15, 2019 •

edited

layershifter Mar 20, 2019 •

edited