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

feat(Menu): allow to pass content to MenuDivider #1009

Merged
merged 7 commits into from
Mar 4, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Mar 4, 2019

Fixes #944.


Why content?

We can introduce an icon shorthand, but what if I want to pass chars? Our Divider component also accepts content/children.

@codecov
Copy link

codecov bot commented Mar 4, 2019

Codecov Report

Merging #1009 into master will decrease coverage by 0.01%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1009      +/-   ##
==========================================
- Coverage   81.38%   81.37%   -0.02%     
==========================================
  Files         673      673              
  Lines        8693     8697       +4     
  Branches     1472     1475       +3     
==========================================
+ Hits         7075     7077       +2     
- Misses       1603     1605       +2     
  Partials       15       15
Impacted Files Coverage Δ
.../themes/teams/components/Menu/menuDividerStyles.ts 22.22% <0%> (-6.35%) ⬇️
packages/react/src/components/Menu/MenuDivider.tsx 100% <100%> (ø) ⬆️

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 26726f1...ee1e998. Read the comment docs.

/**
* Accessibility behavior if overridden by the user.
* @default menuDividerBehavior
*/
accessibility?: Accessibility

icon?: ShorthandValue
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how we are handling the icon prop.. Are we adding both content and icon, or the intention is just the content prop to be added?

Copy link
Member Author

Choose a reason for hiding this comment

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

Cruft, will remove it 👍

{...unhandledProps}
className={classes.root}
/>
<ElementType {...accessibility.attributes.root} {...unhandledProps} className={classes.root}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add this to the ElementType

{...rtlTextContainer.getAttributes({ forElements: [children, content] })}

This will ensure that if the children or content are strings, there will be dir='auto' applied.

CHANGELOG.md Outdated
@@ -41,6 +41,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add `reactionGroup` and `reactionGroupPosition` props to the `ChatMessage` component @mnajdova ([#959](https://github.com/stardust-ui/react/pull/959))
- Set `aria-modal` attribute for both Dialog and Popup with focus trap @sophieH29 ([#995](https://github.com/stardust-ui/react/pull/995))
- Allow arrays as shorthand for the Components containing prop of type `CollectionShorthand` @mnajdova ([#996](https://github.com/stardust-ui/react/pull/996))
- Allow to pass content to `MenuDivider` @layershifter ([#1009](https://github.com/stardust-ui/react/pull/1009))
Copy link
Contributor

@mnajdova mnajdova Mar 4, 2019

Choose a reason for hiding this comment

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

Please replace with:
Allow to pass children and content to MenuDivider @layershifter (#1009)

@layershifter layershifter merged commit d9a0087 into master Mar 4, 2019
@delete-merged-branch delete-merged-branch bot deleted the feat/menu-divider-content branch March 4, 2019 16:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants