feat(Toolbar): support for a popup in menu #1927

lucivpav · 2019-09-12T13:04:02Z

Implements #1894

Scenarios in E2E tests

Popup can be opened using mouse
Popup can be opened using keyboard
Tab when the Popup is focused should not result in hiding the Popup
Click inside Popup should not hide Popup
Popup can be closed when clicking outside of the elements
Click outside of Popup but inside of Menu closes Popup but leaves Menu open

TODO

Try whether it still works in overflow

lucivpav · 2019-09-12T13:04:54Z

docs/src/examples/components/Toolbar/Usage/ToolbarExampleWithTooltip.shorthand.tsx

@@ -67,6 +71,7 @@ const ToolbarExampleShorthand = () => {
                  // Adding tooltipAsLabelBehavior as the ToolbarItems contains only icon
                  return (
                    <Tooltip
+                      key={`${rest.key}-tooltip`} // to avoid errors with keys


to avoid errors with keys

codecov · 2019-09-12T13:16:06Z

Codecov Report

Merging #1927 into master will decrease coverage by 0.02%.
The diff coverage is 40.62%.

@@            Coverage Diff             @@
##           master    #1927      +/-   ##
==========================================
- Coverage   70.42%   70.39%   -0.03%     
==========================================
  Files         889      889              
  Lines        7841     7851      +10     
  Branches     2287     2288       +1     
==========================================
+ Hits         5522     5527       +5     
- Misses       2308     2313       +5     
  Partials       11       11

Impacted Files	Coverage Δ
...kages/react/src/components/Toolbar/ToolbarItem.tsx	`45.16% <33.33%> (-1.14%)`	⬇️
...s/react/src/components/Toolbar/ToolbarMenuItem.tsx	`62.96% <80%> (+2.96%)`	⬆️

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 94c7905...43e3467. Read the comment docs.

docs/src/examples/components/Toolbar/Usage/ToolbarExampleWithTooltip.shorthand.tsx

packages/react/src/components/Toolbar/ToolbarItem.tsx

packages/react/src/lib/accessibility/FocusZone/FocusTrapZone.tsx

jurokapsiar · 2019-09-16T18:15:21Z

docs/src/examples/components/Toolbar/Usage/ToolbarExamplePopupInMenu.shorthand.tsx

+const PopupContent = <Input icon="search" placeholder="Search..." />
+
+const ToolbarExamplePopupInMenu = () => {
+  const [menu1Open, setMenu1Open] = React.useState(false)


can you also add an example (not sure if this or a new file) that shows how they can close the popup after a successful action? for example, if the user selected the item that he wanted to select - in that case both the popup as well as the submenu should close. I want to understand how much state handling would the developer need to do.

Yes, its available here: https://github.com/stardust-ui/react/pull/1927/files#diff-1e0d18e5b10bb6e298c5bdfa5516de67

…ub.com/stardust-ui/react into feat/toolbar-menu-popup

docs/src/examples/components/Toolbar/Usage/ToolbarExamplePopupInMenu.shorthand.tsx

lucivpav · 2019-09-17T15:13:55Z

It is broken again when you click on popup's background. Will be fixed in seperate PR: #1944

jurokapsiar

nice work!

initial commit without new tests

28ffd0f

lucivpav requested review from dvdzkwsk, kuzhelov, layershifter, levithomason, miroslavstastny and mnajdova as code owners September 12, 2019 13:04

lucivpav commented Sep 12, 2019

View reviewed changes

remove useless comments

6b9dc69

lucivpav added the 🚧 WIP label Sep 12, 2019

vercel bot had a problem deploying to staging September 12, 2019 13:08 Failure

add missing import

160db99

vercel bot deployed to staging September 12, 2019 13:16 View deployment

lucivpav commented Sep 12, 2019

View reviewed changes

docs/src/examples/components/Toolbar/Usage/ToolbarExampleWithTooltip.shorthand.tsx Outdated Show resolved Hide resolved

lucivpav commented Sep 12, 2019

View reviewed changes

packages/react/src/components/Toolbar/ToolbarItem.tsx Outdated Show resolved Hide resolved

try to fix failing e2e test

eb442f9

vercel bot temporarily deployed to staging September 12, 2019 16:27 Inactive

remove useless comment

f79a922

vercel bot deployed to staging September 12, 2019 16:30 View deployment

Merge branch 'master' into feat/toolbar-menu-popup

98378ee

vercel bot deployed to staging September 12, 2019 16:44 View deployment

add screener test

a11eb86

vercel bot deployed to staging September 13, 2019 08:49 View deployment

Merge branch 'master' into feat/toolbar-menu-popup

c2d78d0

vercel bot deployed to staging September 13, 2019 09:03 View deployment

fix tabbing inside popup closes the popup

55d1425

vercel bot deployed to staging September 13, 2019 10:10 View deployment

lucivpav commented Sep 13, 2019

View reviewed changes

packages/react/src/lib/accessibility/FocusZone/FocusTrapZone.tsx Outdated Show resolved Hide resolved

fix click inside popup closes menu

3d2e3ae

jurokapsiar reviewed Sep 16, 2019

View reviewed changes

layershifter added 2 commits September 17, 2019 11:13

Merge branches 'feat/toolbar-menu-popup' and 'master' of https://gith…

fe0676c

…ub.com/stardust-ui/react into feat/toolbar-menu-popup

use NestingAuto instead

ae7fbdc

vercel bot deployed to staging September 17, 2019 10:49 View deployment

layershifter reviewed Sep 17, 2019

View reviewed changes

docs/src/examples/components/Toolbar/Usage/ToolbarExamplePopupInMenu.shorthand.tsx Outdated Show resolved Hide resolved

final improvements

317a1a6

vercel bot deployed to staging September 17, 2019 10:58 View deployment

simplify example

9d73c46

vercel bot deployed to staging September 17, 2019 14:59 View deployment

add example where popup has an action that closes the menu

307354c

vercel bot deployed to staging September 17, 2019 15:12 View deployment

lucivpav added 2 commits September 17, 2019 17:19

bring back change to FocusTrapZone

eb4ca70

revert previous commit

f57ab28

lucivpav mentioned this pull request Sep 17, 2019

Popup in Toolbar Menu is closed unintentionally #1944

Closed

add screener steps

50b2c12

vercel bot deployed to staging September 17, 2019 15:31 View deployment

Merge branch 'master' into feat/toolbar-menu-popup

fb95a7c

vercel bot temporarily deployed to staging September 18, 2019 08:52 Inactive

update changelog

889c835

vercel bot deployed to staging September 18, 2019 08:55 View deployment

layershifter approved these changes Sep 18, 2019

View reviewed changes

jurokapsiar approved these changes Sep 18, 2019

View reviewed changes

Merge branch 'master' into feat/toolbar-menu-popup

090053d

vercel bot deployed to staging September 18, 2019 13:18 View deployment

fix prettier

43e3467

vercel bot deployed to staging September 18, 2019 13:28 View deployment

lucivpav merged commit 5cce993 into master Sep 18, 2019

lucivpav deleted the feat/toolbar-menu-popup branch September 18, 2019 13:58

jurokapsiar mentioned this pull request Sep 19, 2019

Support Popup in Toolbar menu #1894

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Toolbar): support for a popup in menu #1927

feat(Toolbar): support for a popup in menu #1927

lucivpav commented Sep 12, 2019 •

edited

lucivpav Sep 12, 2019

codecov bot commented Sep 12, 2019 •

edited

jurokapsiar Sep 16, 2019

lucivpav Sep 18, 2019

lucivpav commented Sep 17, 2019 •

edited

jurokapsiar left a comment

feat(Toolbar): support for a popup in menu #1927

feat(Toolbar): support for a popup in menu #1927

Conversation

lucivpav commented Sep 12, 2019 • edited

Scenarios in E2E tests

TODO

lucivpav Sep 12, 2019

Choose a reason for hiding this comment

codecov bot commented Sep 12, 2019 • edited

Codecov Report

jurokapsiar Sep 16, 2019

Choose a reason for hiding this comment

lucivpav Sep 18, 2019

Choose a reason for hiding this comment

lucivpav commented Sep 17, 2019 • edited

jurokapsiar left a comment

Choose a reason for hiding this comment

lucivpav commented Sep 12, 2019 •

edited

codecov bot commented Sep 12, 2019 •

edited

lucivpav commented Sep 17, 2019 •

edited