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

[NcActions] doesn't support non-direct NcAction* usage #5171

Open
ShGKme opened this issue Jan 29, 2024 · 4 comments
Open

[NcActions] doesn't support non-direct NcAction* usage #5171

ShGKme opened this issue Jan 29, 2024 · 4 comments
Assignees
Labels
1. to develop Accepted and waiting to be taken care of feature: actions Related to the actions components

Comments

@ShGKme
Copy link
Contributor

ShGKme commented Jan 29, 2024

This works:

<NcActions>
  <NcAction* />
  <NcAction* />
</NcActions>

This doesn't:

<NcActions>
  <MyActionWrapper* /> -- Renders <NcAction* />
  <MyActionWrapper* />
</NcActions>

The problem is that in NcActions we manually take actions from slot in render function.
And some parts check, what exactly action do we have.

For example, here (1)

isValidSingleAction(action) {
return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].includes(this.getActionName(action))
},

and here (2)

const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio']
const textInputActions = ['NcActionInput', 'NcActionTextEditable']
const linkActions = ['NcActionLink', 'NcActionRouter']
const hasTextInputAction = menuActions.some(action => textInputActions.includes(this.getActionName(action)))
const hasMenuItemAction = menuActions.some(action => menuItemsActions.includes(this.getActionName(action)))
const hasLinkAction = menuActions.some(action => linkActions.includes(this.getActionName(action)))

Because as an action name we take vnode's component instance's name:

getActionName(action) {
return action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag
},

For MyActionWrapper it will be MyActionWrapper even if it renders <NcActionButton>.

While (1) only results in broken inline feature, (2) now breaks a whole <NcActions> when we use custom wrappers.

The complex part here is that in Vue when a parent is being rendered, it has no child yet. So we cannot just make recursive getActionName. So during NcActions rendering we have no idea what child MyActionWrapper way actually be.

@ShGKme ShGKme added 1. to develop Accepted and waiting to be taken care of feature: actions Related to the actions components labels Jan 29, 2024
@ShGKme ShGKme self-assigned this Jan 29, 2024
@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 29, 2024

The current idea:

  1. Check for the action name recursively
  2. If no NcAction* was found - mark instance as not-ready and schedule a $forceUpdate

@JuliaKirschenheuter
Copy link
Contributor

@ShGKme thanks a lot for this fix! Could this issue be closed?

@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 30, 2024

@ShGKme thanks a lot for this fix! Could this issue be closed?

Unfortunately, no, until Vue 3 migration.

My hotfix only brings back an old behavior when it "works in general", but has a number of limitations, including in a11y.

I only fixed a new "completely broken" state.

For a full-working solution, we need to either search for NcAction* components recursively or find some other solution.

@ShGKme
Copy link
Contributor Author

ShGKme commented Jan 30, 2024

UPDATE: Vue 3 has the same limitation

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of feature: actions Related to the actions components
Projects
None yet
Development

No branches or pull requests

2 participants