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

feat(Toolbar): add popup prop #1480

Merged
merged 10 commits into from
Jun 13, 2019
Merged

feat(Toolbar): add popup prop #1480

merged 10 commits into from
Jun 13, 2019

Conversation

miroslavstastny
Copy link
Member

@miroslavstastny miroslavstastny commented Jun 10, 2019

Closes #1410.

A Toolbar item can now have a popup:
image

API

popup prop was added to the ToolbarItem. It is a shorthand for Popup component, accepting all config valid for Popup with the following exceptions:

  • accessibility defaults to popupFocusTrapBehavior (but can be overriden)
  • children prop cannot be used, it is always reset by the ToolbarItem
  • trigger prop cannot be used, it is force-set to the ToolbarItem's button

Toolbar is a controlled component, it does not internally handle items' state. It is consumer's responsibility to handle MenuItem:active prop based on the Popup state.

@codecov
Copy link

codecov bot commented Jun 10, 2019

Codecov Report

Merging #1480 into master will decrease coverage by 0.01%.
The diff coverage is 45.45%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1480      +/-   ##
==========================================
- Coverage   73.45%   73.44%   -0.02%     
==========================================
  Files         822      822              
  Lines        6183     6190       +7     
  Branches     1793     1795       +2     
==========================================
+ Hits         4542     4546       +4     
- Misses       1636     1639       +3     
  Partials        5        5
Impacted Files Coverage Δ
packages/react/src/types.ts 50% <ø> (ø) ⬆️
packages/react/src/components/Popup/Popup.tsx 67.94% <100%> (+0.2%) ⬆️
packages/react-proptypes/src/index.ts 33.8% <16.66%> (-0.02%) ⬇️
...kages/react/src/components/Toolbar/ToolbarItem.tsx 85% <75%> (-3.24%) ⬇️

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 47c2ab5...a566758. Read the comment docs.

Copy link
Collaborator

@bmdalex bmdalex left a comment

Choose a reason for hiding this comment

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

looks good generally, couple of comments

Copy link
Contributor

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Looks good 👍 ! Don't forget to add changelog entry.

@miroslavstastny miroslavstastny merged commit 6be96b6 into master Jun 13, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat/toolbar-popup branch June 13, 2019 07:41
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.

Toolbar - popup
5 participants