From f502143ee6ab3ba5ab1fcb00dadac7be4003dd57 Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Fri, 26 Jan 2024 12:19:45 +0100 Subject: [PATCH 1/5] refactor(NcActions): reuse getActionName method Signed-off-by: Grigorii K. Shartsev --- src/components/NcActions/NcActions.vue | 35 +++++++++++++++----------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/src/components/NcActions/NcActions.vue b/src/components/NcActions/NcActions.vue index fe36aa8694..9b4f72e982 100644 --- a/src/components/NcActions/NcActions.vue +++ b/src/components/NcActions/NcActions.vue @@ -1019,16 +1019,25 @@ export default { }, methods: { + /** + * Get the name of the action component + * + * @param {import('vue').VNode} action - a vnode with a NcAction* component instance + * @return {string} the name of the action component + */ + getActionName(action) { + return action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag + }, + /** * Do we have exactly one Action and * is it allowed as a standalone element? * - * @param {Array} action The action to check + * @param {import('vue').VNode} action The action to check * @return {boolean} */ isValidSingleAction(action) { - const componentName = action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag - return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].includes(componentName) + return ['NcActionButton', 'NcActionLink', 'NcActionRouter'].includes(this.getActionName(action)) }, /** @@ -1236,19 +1245,20 @@ export default { * This also ensure that we don't get 'text' elements, which would * become problematic later on. */ - const actions = (this.$slots.default || []).filter( - action => action?.componentOptions?.tag || action?.componentOptions?.Ctor?.extendOptions?.name, - ) + const actions = (this.$slots.default || []).filter(action => this.getActionName(action)) - const getActionName = (action) => action?.componentOptions?.Ctor?.extendOptions?.name ?? action?.componentOptions?.tag + // Check that we have at least one action + if (actions.length === 0) { + return + } const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio'] const textInputActions = ['NcActionInput', 'NcActionTextEditable'] const linkActions = ['NcActionLink', 'NcActionRouter'] - const hasTextInputAction = actions.some(action => textInputActions.includes(getActionName(action))) - const hasMenuItemAction = actions.some(action => menuItemsActions.includes(getActionName(action))) - const hasLinkAction = actions.some(action => linkActions.includes(getActionName(action))) + const hasTextInputAction = actions.some(action => textInputActions.includes(this.getActionName(action))) + const hasMenuItemAction = actions.some(action => menuItemsActions.includes(this.getActionName(action))) + const hasLinkAction = actions.some(action => linkActions.includes(this.getActionName(action))) // We consider the NcActions to have role="menu" if it consists some button-like action and not text inputs this.isSemanticMenu = hasMenuItemAction && !hasTextInputAction @@ -1272,11 +1282,6 @@ export default { inlineActions = [] } - // Check that we have at least one action - if (actions.length === 0) { - return - } - /** * Render the provided action * From b66bfc87756f82975d70c1954ec579a7771c94bf Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Fri, 26 Jan 2024 12:22:25 +0100 Subject: [PATCH 2/5] refactor(NcActions): better name inline and menu action items vars - `inlineActions` -> `validInlineActions` like it is said in filter, because not all of them are to be inline - `renderedInlineActions` -> `inlineActions` because they are not rendered yet and to be consistent with `menuActions` Signed-off-by: Grigorii K. Shartsev --- src/components/NcActions/NcActions.vue | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/NcActions/NcActions.vue b/src/components/NcActions/NcActions.vue index 9b4f72e982..4c7b495812 100644 --- a/src/components/NcActions/NcActions.vue +++ b/src/components/NcActions/NcActions.vue @@ -1276,10 +1276,10 @@ export default { /** * Filter and list actions that are allowed to be displayed inline */ - let inlineActions = actions.filter(this.isValidSingleAction) - if (this.forceMenu && inlineActions.length > 0 && this.inline > 0) { + let validInlineActions = actions.filter(this.isValidSingleAction) + if (this.forceMenu && validInlineActions.length > 0 && this.inline > 0) { Vue.util.warn('Specifying forceMenu will ignore any inline actions rendering.') - inlineActions = [] + validInlineActions = [] } /** @@ -1462,8 +1462,8 @@ export default { * If we have a single action only and didn't force a menu, * we render the action as a standalone button */ - if (actions.length === 1 && inlineActions.length === 1 && !this.forceMenu) { - return renderInlineAction(inlineActions[0]) + if (actions.length === 1 && validInlineActions.length === 1 && !this.forceMenu) { + return renderInlineAction(validInlineActions[0]) } // If we completely re-render the children @@ -1481,10 +1481,10 @@ export default { /** * If we some inline actions to render, render them, then the menu */ - if (inlineActions.length > 0 && this.inline > 0) { - const renderedInlineActions = inlineActions.slice(0, this.inline) + if (validInlineActions.length > 0 && this.inline > 0) { + const inlineActions = validInlineActions.slice(0, this.inline) // Filter already rendered actions - const menuActions = actions.filter(action => !renderedInlineActions.includes(action)) + const menuActions = actions.filter(action => !inlineActions.includes(action)) return h('div', { class: [ @@ -1494,7 +1494,7 @@ export default { }, [ // Render inline actions - ...renderedInlineActions.map(renderInlineAction), + ...inlineActions.map(renderInlineAction), // render the rest within the popover menu menuActions.length > 0 ? h('div', From dbee0031942fa573598ae38ebb2a6e8f676023fa Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Fri, 26 Jan 2024 12:28:07 +0100 Subject: [PATCH 3/5] refactor(NcActions): move separating inline/menu items code in one place Signed-off-by: Grigorii K. Shartsev --- src/components/NcActions/NcActions.vue | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/components/NcActions/NcActions.vue b/src/components/NcActions/NcActions.vue index 4c7b495812..28309988f1 100644 --- a/src/components/NcActions/NcActions.vue +++ b/src/components/NcActions/NcActions.vue @@ -1274,13 +1274,25 @@ export default { : 'true' /** - * Filter and list actions that are allowed to be displayed inline + * Separate the actions into inline and menu actions + */ + + /** + * @type {import('vue').VNode[]} */ let validInlineActions = actions.filter(this.isValidSingleAction) if (this.forceMenu && validInlineActions.length > 0 && this.inline > 0) { Vue.util.warn('Specifying forceMenu will ignore any inline actions rendering.') validInlineActions = [] } + /** + * @type {import('vue').VNode[]} + */ + const inlineActions = validInlineActions.slice(0, this.inline) + /** + * @type {import('vue').VNode[]} + */ + const menuActions = actions.filter(action => !inlineActions.includes(action)) /** * Render the provided action @@ -1463,7 +1475,7 @@ export default { * we render the action as a standalone button */ if (actions.length === 1 && validInlineActions.length === 1 && !this.forceMenu) { - return renderInlineAction(validInlineActions[0]) + return renderInlineAction(actions[0]) } // If we completely re-render the children @@ -1481,10 +1493,7 @@ export default { /** * If we some inline actions to render, render them, then the menu */ - if (validInlineActions.length > 0 && this.inline > 0) { - const inlineActions = validInlineActions.slice(0, this.inline) - // Filter already rendered actions - const menuActions = actions.filter(action => !inlineActions.includes(action)) + if (inlineActions.length > 0 && this.inline > 0) { return h('div', { class: [ From bb05b7b7f1acce832f9b6e0c54b6c71ae4d4d2af Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Fri, 26 Jan 2024 12:34:05 +0100 Subject: [PATCH 4/5] fix(NcActions): don't include inline actions in menu semantics Signed-off-by: Grigorii K. Shartsev --- src/components/NcActions/NcActions.vue | 48 +++++++++++++++----------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/src/components/NcActions/NcActions.vue b/src/components/NcActions/NcActions.vue index 28309988f1..3241e42879 100644 --- a/src/components/NcActions/NcActions.vue +++ b/src/components/NcActions/NcActions.vue @@ -4,6 +4,7 @@ - @author John Molakvoæ - @author Marco Ambrosini - @author Raimund Schlüßler + - @author Grigorii K. Shartsev - - @license GNU AGPL version 3 or any later version - @@ -1252,27 +1253,6 @@ export default { return } - const menuItemsActions = ['NcActionButton', 'NcActionButtonGroup', 'NcActionCheckbox', 'NcActionRadio'] - const textInputActions = ['NcActionInput', 'NcActionTextEditable'] - const linkActions = ['NcActionLink', 'NcActionRouter'] - - const hasTextInputAction = actions.some(action => textInputActions.includes(this.getActionName(action))) - const hasMenuItemAction = actions.some(action => menuItemsActions.includes(this.getActionName(action))) - const hasLinkAction = actions.some(action => linkActions.includes(this.getActionName(action))) - - // We consider the NcActions to have role="menu" if it consists some button-like action and not text inputs - this.isSemanticMenu = hasMenuItemAction && !hasTextInputAction - // We consider the NcActions to be navigation if it consists some link-like action - this.isSemanticNavigation = hasLinkAction && !hasMenuItemAction && !hasTextInputAction - // If it is not a menu and not a navigation, it is a popover with items: a form or just a text - this.isSemanticPopoverLike = !this.isSemanticMenu && !this.isSemanticNavigation - - const popupRole = this.isSemanticMenu - ? 'menu' - : hasTextInputAction - ? 'dialog' - : 'true' - /** * Separate the actions into inline and menu actions */ @@ -1294,6 +1274,32 @@ export default { */ const menuActions = actions.filter(action => !inlineActions.includes(action)) + /** + * Determine what kind of menu we have. + * It defines focus behavior and a11y. + */ + + 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))) + + // We consider the NcActions to have role="menu" if it consists some button-like action and not text inputs + this.isSemanticMenu = hasMenuItemAction && !hasTextInputAction + // We consider the NcActions to be navigation if it consists some link-like action + this.isSemanticNavigation = hasLinkAction && !hasMenuItemAction && !hasTextInputAction + // If it is not a menu and not a navigation, it is a popover with items: a form or just a text + this.isSemanticPopoverLike = !this.isSemanticMenu && !this.isSemanticNavigation + + const popupRole = this.isSemanticMenu + ? 'menu' + : hasTextInputAction + ? 'dialog' + : 'true' + /** * Render the provided action * From f26764af68218f62804eb799849f993d08cf6698 Mon Sep 17 00:00:00 2001 From: "Grigorii K. Shartsev" Date: Fri, 26 Jan 2024 21:06:23 +0100 Subject: [PATCH 5/5] fix(NcActions): handle text-only popup keyboard navigation - Add new semantic type - tooltip (popup with text only, without interactive elements) - Simplify semantic types logic - Manually close tooltips on blur Signed-off-by: Grigorii K. Shartsev --- src/components/NcActions/NcActions.vue | 82 ++++++++++++++++++-------- 1 file changed, 56 insertions(+), 26 deletions(-) diff --git a/src/components/NcActions/NcActions.vue b/src/components/NcActions/NcActions.vue index 3241e42879..d91cde1e6a 100644 --- a/src/components/NcActions/NcActions.vue +++ b/src/components/NcActions/NcActions.vue @@ -698,8 +698,8 @@ export default {

-

Popover

- Has any elements, including text input element, or no buttons. +

Dialog

+ Includes data input elements

@@ -715,6 +715,19 @@ export default {

+ +

Toolip

+ Has only text and not interactive elements +

+ + + View profile + + + Local time: 10:12 + + +

@@ -794,7 +807,6 @@ p { } ``` -