refactor(components): refactor menu #3639
Conversation
sxzz
force-pushed
the
refactor/menu
branch
2 times, most recently
from
65ffdef
to
65bd98f
Compare
|
|
sxzz
force-pushed
the
refactor/menu
branch
2 times, most recently
from
8341c64
to
26b7a13
Compare
|
I'm looking into this now |
| setup() { | ||
| const instance = getCurrentInstance()! | ||
| const menu = inject<MenuProvider>('rootMenu') | ||
| if (!menu) throwError(COMPONENT_NAME, 'can not inject root menu') |
There was a problem hiding this comment.
Minor: I know this file wasn't originated from you, but the error message here is confusing we might need to change it to:
needs be nested under
There was a problem hiding this comment.
We can always create a follow up issue for fixing.
| }, | ||
| } as BaseTransitionProps<HTMLElement> as TransitionProps |
There was a problem hiding this comment.
@JeremyWuuuuu I added type here.
| click: (item: MenuItemRegistered) => | ||
| isString(item.index) && Array.isArray(item.indexPath), |
There was a problem hiding this comment.
Do we really need to implement the actual method here? Since I thought this should only be available for marking the type.
There was a problem hiding this comment.
This is to validate the emit. Since I wrote it, just keep it, and it won't affect production.
| const rootMenu = inject<MenuProvider>('rootMenu') | ||
| if (!rootMenu) throwError(COMPONENT_NAME, 'can not inject root menu') |
There was a problem hiding this comment.
Non-blocking: Maybe we can make rootMenu a symbol as we did for elFormKey, and extract them into @element-plus/tokens
This can be changed in a followup PR. ;)
There was a problem hiding this comment.
Yes, I tried, but @element-plus/token build script seems broken. So I need fix build script first.
There was a problem hiding this comment.
Then how did the forms and buttons working 😕
There was a problem hiding this comment.
TypeScript emit errors.
|
Most of my comments were not blocking but I think the changes against |
|
@JeremyWuuuuu Done. |
Please make sure these boxes are checked before submitting your PR, thank you!
devbranch.