-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
feat: add rtl menu #2056
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
feat: add rtl menu #2056
Conversation
|
I need you update |
|
when you done, i will rebase this branch |
components/menu/MenuItem.jsx
Outdated
| title: title || (level === 1 ? $slots.default : ''), | ||
| }; | ||
| const siderCollapsed = this.layoutSiderContext.sCollapsed; | ||
| const direction = this.configProvider.direction; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这里用解构比较好吧
components/menu/index.jsx
Outdated
| const { collapsedWidth } = layoutSiderContext; | ||
| const { getPopupContainer: getContextPopupContainer } = this.configProvider; | ||
| const { prefixCls: customizePrefixCls, theme, getPopupContainer } = this.$props; | ||
| const getPrefixCls = this.configProvider.getPrefixCls; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Amour1688 之前有一个 getPrefixCls 这么写了,然后我就统一这样写了😂😂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
如果要用解构,还要改之前的,所以就没动,保持统一。
看以后是不是需要一起改
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
后面改的都用结构吧
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
嗯,新写的我都用了解构了,之前的我都改一下
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tangjinzhou has fixed
|
Close first, reopen when necessary. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
First of all, thank you for your contribution! 😄
New feature please send pull request to feature branch, and rest to master branch. Pull request will be merged after one of collaborators approve. Please makes sure that these form are filled before submitting your pull request, thank you!
[中文版模板 / Chinese template]
This is a ...
What's the background?
API Realization (Optional if not new feature)
What's the effect? (Optional if not new feature)
Changelog description (Optional if not new feature)
Self Check before Merge
Additional Plan? (Optional if not new feature)