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

ElMenu: update activedIndex/openedMenus when items changes, fix #2670 #3671

Closed
wants to merge 2 commits into from

Conversation

reverland
Copy link
Contributor

@reverland reverland commented Mar 22, 2017

The current problem is, many people need to dynamic generate menu-items. however, when one watch default-active, the menu may still not generated, when the menu generated, one need to change the value the default-active is and turn it back to make the watch work. eg.

this.currentPath = 'something else'
this.$nextTick(_ => {
  this.currentPath = 'the path you like'
})

its quite a dirty workaround. So maybe menu component should watch items change and apply updateActive automaticly.

@reverland
Copy link
Contributor Author

Just for discussion, i'm not sure it should be designed like this.

@baiyaaaaa
Copy link
Contributor

感觉解决方案并不靠谱,这样会导致其他的问题,每次menu-item数量变化不应该都重新设置一次展开项,既然你的数据是异步的,那展开项的变化应该由用户自己去设置 default-openeds

@reverland
Copy link
Contributor Author

额,这会导致什么问题?

default-opened不需要在menu-item数据变化后也这样更改一次再改回去么?

但如果default-opened能在menu-item数据变化时时刻保持更新,而不像default-active那样第一次没匹配上第二次能匹配上但数据没变化没触发watch这种比较奇怪的现象的话,就关闭PR吧

我不大有时间去仔细思考和研究这个问题,但default-active在异步下的问题,也许能在文档中说明下。

@baiyaaaaa

@baiyaaaaa
Copy link
Contributor

@reverland
我一直在思考这个问题,我的想法是假如你的 meun-item slot 是异步生成的,那是不是应该在获取到数据时再去设置你的 default-opened 和 default-active,这样就能触发 watch 了。

当你数据还没有时,是不应该去设置一个预设的 default-opened 和 default-active 的。

@reverland
Copy link
Contributor Author

@baiyaaaaa nice, 听起来不错

每次数据变化就强制重新生成menu

我觉得watch也不用了233

@baiyaaaaa
Copy link
Contributor

@reverland 对的,所以这个我先关掉了,#2670 issue 提交者已经这样解决了

@baiyaaaaa baiyaaaaa closed this May 31, 2017
lq782655835 added a commit to lq782655835/element that referenced this pull request Mar 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants