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

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

Merged
merged 32 commits into from
Sep 18, 2019
Merged

Conversation

lucivpav
Copy link
Contributor

@lucivpav lucivpav commented Sep 12, 2019

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

@@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to avoid errors with keys

@codecov
Copy link

codecov bot commented Sep 12, 2019

Codecov Report

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

Impacted file tree graph

@@            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.

@vercel vercel bot temporarily deployed to staging September 12, 2019 16:27 Inactive
const PopupContent = <Input icon="search" placeholder="Search..." />

const ToolbarExamplePopupInMenu = () => {
const [menu1Open, setMenu1Open] = React.useState(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lucivpav
Copy link
Contributor Author

lucivpav commented Sep 17, 2019

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

@vercel vercel bot temporarily deployed to staging September 18, 2019 08:52 Inactive
Copy link
Contributor

@jurokapsiar jurokapsiar left a comment

Choose a reason for hiding this comment

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

nice work!

@lucivpav lucivpav merged commit 5cce993 into master Sep 18, 2019
@lucivpav lucivpav deleted the feat/toolbar-menu-popup branch September 18, 2019 13:58
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.

3 participants