Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

icon-menu using popover #2149

Merged
merged 1 commit into from
Nov 26, 2015
Merged

icon-menu using popover #2149

merged 1 commit into from
Nov 26, 2015

Conversation

chrismcv
Copy link
Contributor

Reworked from #1845
This means an icon-menu in a dialog can break outside it if needs be

closeOnItemTouchTap: React.PropTypes.bool,
iconButtonElement: React.PropTypes.element.isRequired,
iconStyle: React.PropTypes.object,
openDirection: PropTypes.corners,
menuOverlapsIcon: React.PropTypes.bool,
Copy link
Member

Choose a reason for hiding this comment

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

Seems to me that you are not using menuOverlapsIcon

@oliviertassinari
Copy link
Member

I have noticed one issue, the Menu with various open directions examples of the doc have the same opening direction.

@oliviertassinari oliviertassinari added PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI and removed Review: review-needed labels Nov 24, 2015
@chrismcv
Copy link
Contributor Author

@oliviertassinari this one is good to review again

@oliviertassinari oliviertassinari added Review: review-needed and removed PR: needs revision The pull request can't be merged. More details is available on the code review or fails in the CI labels Nov 26, 2015
@@ -279,8 +279,9 @@ export default class IconMenus extends React.Component {
<div>
<IconMenu
iconButtonElement={mapsButtonElement}
maxHeight={272}
openDirection="bottom-right"
Copy link
Member

Choose a reason for hiding this comment

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

Could you remove openDirection form the doc and add the new properties?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for updating the docs, I had hard time before finding this

@oliviertassinari
Copy link
Member

@chrismcv It's almost ready to be merged 👍.

@chrismcv
Copy link
Contributor Author

@oliviertassinari should be good now

@oliviertassinari
Copy link
Member

@chrismcv Thanks
@shaurya947 This introduce a breaking change. openDirection was removed, we now should use anchorOrigin and targetOrigin.

oliviertassinari added a commit that referenced this pull request Nov 26, 2015
[icon-menu] Use popover component
@oliviertassinari oliviertassinari merged commit 2ab0723 into mui:master Nov 26, 2015
@oliviertassinari
Copy link
Member

I have just noticed one issue. I feel like there is a delay between a tap on an item and the closing of the menu. Do you confirm?

@oliviertassinari
Copy link
Member

It may be linked to touchTapCloseDelay. I will investigat tomorrow

@oliviertassinari
Copy link
Member

Actually, it takes time to disappear when the component is unmounted.
Any idea how we can solve this?

@chrismcv
Copy link
Contributor Author

I think the issue is that the Level 1 menu is starting to animate its close (caused by ClickAwayAble) at the same time as the click on the nested one... haven't come up with a solution yet...

@shaurya947
Copy link
Contributor

@oliviertassinari gotcha. Next release we'll bump the minor version.

@chrismcv chrismcv deleted the icon-menu branch February 1, 2016 21:13
@zannager zannager added component: menu This is the name of the generic UI component, not the React module! component: Popover The React component. package: icons Specific to @mui/icons labels Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module! component: Popover The React component. package: icons Specific to @mui/icons
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants