SubMenu: Fix expanding sub menu items on touch devices (#93208)

* click more in mobile phone

* use stopPropagation stop event

* delete log

* delete inspect click and add stopPropagation

* bug fix

* fix unit tests

* fix e2e tests

* fix old-arch tests

---------

Co-authored-by: Ashley Harrison <ashley.harrison@grafana.com>
This commit is contained in:
jackyin 2024-10-07 22:22:18 +08:00 committed by GitHub
parent 1a3365d99b
commit 63bdbb67fc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 21 additions and 29 deletions

View File

@ -49,7 +49,8 @@ export const importDashboard = (dashboardToImport: Dashboard, queryTimeout?: num
dashboardToImport.panels.forEach((panel) => { dashboardToImport.panels.forEach((panel) => {
// Look at the json data // Look at the json data
e2e.components.Panels.Panel.menu(panel.title).click({ force: true }); // force click because menu is hidden and show on hover e2e.components.Panels.Panel.menu(panel.title).click({ force: true }); // force click because menu is hidden and show on hover
e2e.components.Panels.Panel.menuItems('Inspect').should('be.visible').click(); e2e.components.Panels.Panel.menuItems('Inspect').trigger('mouseover', { force: true });
e2e.components.Panels.Panel.menuItems('Data').click({ force: true });
e2e.components.Tab.title('JSON').should('be.visible').click(); e2e.components.Tab.title('JSON').should('be.visible').click();
e2e.components.PanelInspector.Json.content().should('be.visible').contains('Panel JSON').click({ force: true }); e2e.components.PanelInspector.Json.content().should('be.visible').contains('Panel JSON').click({ force: true });
e2e.components.Select.option().should('be.visible').contains('Panel data').click(); e2e.components.Select.option().should('be.visible').contains('Panel data').click();

View File

@ -34,8 +34,10 @@ describe('Inspect drawer tests', () => {
e2e.flows.openDashboard({ uid: 'wfTJJL5Wz' }); e2e.flows.openDashboard({ uid: 'wfTJJL5Wz' });
// testing opening inspect drawer directly by clicking on Inspect in header menu e2e.components.Panels.Panel.title(PANEL_UNDER_TEST).scrollIntoView().should('be.visible');
e2e.flows.openPanelMenuItem(e2e.flows.PanelMenuItems.Inspect, PANEL_UNDER_TEST); e2e.components.Panels.Panel.menu(PANEL_UNDER_TEST).click({ force: true }); // force click because menu is hidden and show on hover
e2e.components.Panels.Panel.menuItems('Inspect').trigger('mouseover', { force: true });
e2e.components.Panels.Panel.menuItems('Data').click({ force: true });
expectDrawerTabsAndContent(); expectDrawerTabsAndContent();

View File

@ -49,7 +49,8 @@ export const importDashboard = (dashboardToImport: Dashboard, queryTimeout?: num
dashboardToImport.panels.forEach((panel) => { dashboardToImport.panels.forEach((panel) => {
// Look at the json data // Look at the json data
e2e.components.Panels.Panel.menu(panel.title).click({ force: true }); // force click because menu is hidden and show on hover e2e.components.Panels.Panel.menu(panel.title).click({ force: true }); // force click because menu is hidden and show on hover
e2e.components.Panels.Panel.menuItems('Inspect').should('be.visible').click(); e2e.components.Panels.Panel.menuItems('Inspect').trigger('mouseover', { force: true });
e2e.components.Panels.Panel.menuItems('Data').click({ force: true });
e2e.components.Tab.title('JSON').should('be.visible').click(); e2e.components.Tab.title('JSON').should('be.visible').click();
e2e.components.PanelInspector.Json.content().should('be.visible'); e2e.components.PanelInspector.Json.content().should('be.visible');
e2e.components.ReactMonacoEditor.editorLazy().should('be.visible'); e2e.components.ReactMonacoEditor.editorLazy().should('be.visible');

View File

@ -34,8 +34,10 @@ describe('Inspect drawer tests', () => {
e2e.flows.openDashboard({ uid: 'wfTJJL5Wz' }); e2e.flows.openDashboard({ uid: 'wfTJJL5Wz' });
// testing opening inspect drawer directly by clicking on Inspect in header menu e2e.components.Panels.Panel.title(PANEL_UNDER_TEST).scrollIntoView().should('be.visible');
e2e.flows.openPanelMenuItem(e2e.flows.PanelMenuItems.Inspect, PANEL_UNDER_TEST); e2e.components.Panels.Panel.menu(PANEL_UNDER_TEST).click({ force: true }); // force click because menu is hidden and show on hover
e2e.components.Panels.Panel.menuItems('Inspect').trigger('mouseover', { force: true });
e2e.components.Panels.Panel.menuItems('Data').click({ force: true });
expectDrawerTabsAndContent(); expectDrawerTabsAndContent();

View File

@ -152,7 +152,13 @@ export const MenuItem = React.memo(
className={itemStyle} className={itemStyle}
rel={target === '_blank' ? 'noopener noreferrer' : undefined} rel={target === '_blank' ? 'noopener noreferrer' : undefined}
href={url} href={url}
onClick={onClick} onClick={(event) => {
if (hasSubMenu && !isSubMenuOpen) {
event.preventDefault();
event.stopPropagation();
}
onClick?.(event);
}}
onMouseEnter={onMouseEnter} onMouseEnter={onMouseEnter}
onMouseLeave={onMouseLeave} onMouseLeave={onMouseLeave}
onKeyDown={handleKeys} onKeyDown={handleKeys}

View File

@ -1,4 +1,4 @@
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react'; import { act, render, screen, waitFor } from '@testing-library/react';
import userEvent from '@testing-library/user-event'; import userEvent from '@testing-library/user-event';
import { cloneDeep } from 'lodash'; import { cloneDeep } from 'lodash';
import { useParams } from 'react-router-dom-v5-compat'; import { useParams } from 'react-router-dom-v5-compat';
@ -206,7 +206,7 @@ describe('DashboardScenePage', () => {
const inspectMenuItem = await screen.findAllByText('Inspect'); const inspectMenuItem = await screen.findAllByText('Inspect');
act(() => fireEvent.click(inspectMenuItem[0])); await userEvent.click(inspectMenuItem[0]);
expect(await screen.findByText('Inspect: Panel B')).toBeInTheDocument(); expect(await screen.findByText('Inspect: Panel B')).toBeInTheDocument();

View File

@ -70,7 +70,6 @@ describe('getPanelMenu()', () => {
}, },
{ {
"iconClassName": "info-circle", "iconClassName": "info-circle",
"onClick": [Function],
"shortcut": "i", "shortcut": "i",
"subMenu": [ "subMenu": [
{ {
@ -83,7 +82,6 @@ describe('getPanelMenu()', () => {
}, },
{ {
"iconClassName": "cube", "iconClassName": "cube",
"onClick": [Function],
"subMenu": [ "subMenu": [
{ {
"onClick": [Function], "onClick": [Function],
@ -343,7 +341,6 @@ describe('getPanelMenu()', () => {
}, },
{ {
"iconClassName": "info-circle", "iconClassName": "info-circle",
"onClick": [Function],
"shortcut": "i", "shortcut": "i",
"subMenu": [ "subMenu": [
{ {
@ -356,7 +353,6 @@ describe('getPanelMenu()', () => {
}, },
{ {
"iconClassName": "cube", "iconClassName": "cube",
"onClick": [Function],
"subMenu": [ "subMenu": [
{ {
"href": undefined, "href": undefined,

View File

@ -73,10 +73,6 @@ export function getPanelMenu(
}); });
}; };
const onMore = (event: React.MouseEvent) => {
event.preventDefault();
};
const onDuplicatePanel = (event: React.MouseEvent) => { const onDuplicatePanel = (event: React.MouseEvent) => {
event.preventDefault(); event.preventDefault();
duplicatePanel(dashboard, panel); duplicatePanel(dashboard, panel);
@ -176,17 +172,6 @@ export function getPanelMenu(
type: 'submenu', type: 'submenu',
text: t('panel.header-menu.inspect', `Inspect`), text: t('panel.header-menu.inspect', `Inspect`),
iconClassName: 'info-circle', iconClassName: 'info-circle',
onClick: (e: React.MouseEvent<HTMLElement>) => {
const currentTarget = e.currentTarget;
const target = e.target;
if (
target === currentTarget ||
(target instanceof HTMLElement && target.closest('[role="menuitem"]') === currentTarget)
) {
onInspectPanel();
}
},
shortcut: 'i', shortcut: 'i',
subMenu: inspectMenu, subMenu: inspectMenu,
}); });
@ -323,7 +308,6 @@ export function getPanelMenu(
text: t('panel.header-menu.more', `More...`), text: t('panel.header-menu.more', `More...`),
iconClassName: 'cube', iconClassName: 'cube',
subMenu, subMenu,
onClick: onMore,
}); });
} }