PanelChrome: Fixes issues with hover header and resizing panel above (#71040)

* PanelChrome: Fixes issues with hover header and sizing panel above

* Update

* Make panel be focusable

* Fix tooltip when using keyboard nav

* Re-render grid when layout change to have dom positions match absolute css positions

* Fix clicking panel leaves hover header open
This commit is contained in:
Torkel Ödegaard 2023-07-05 14:17:51 +02:00 committed by GitHub
parent f02e1548d7
commit 7252c6dd91
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 41 additions and 48 deletions

View File

@ -1,5 +1,5 @@
import { css, cx } from '@emotion/css'; import { css, cx } from '@emotion/css';
import React, { ReactElement, useCallback, useRef, useState } from 'react'; import React, { ReactElement, useCallback, useRef } from 'react';
import { GrafanaTheme2 } from '@grafana/data'; import { GrafanaTheme2 } from '@grafana/data';
import { selectors as e2eSelectors } from '@grafana/e2e-selectors'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors';
@ -31,18 +31,12 @@ export function HoverWidget({ menu, title, dragClass, children, offset = -32, on
draggableRef.current?.releasePointerCapture(e.pointerId); draggableRef.current?.releasePointerCapture(e.pointerId);
}, []); }, []);
const [menuOpen, setMenuOpen] = useState(false);
if (children === undefined || React.Children.count(children) === 0) { if (children === undefined || React.Children.count(children) === 0) {
return null; return null;
} }
return ( return (
<div <div className={cx(styles.container, 'show-on-hover')} style={{ top: offset }} data-testid={selectors.container}>
className={cx(styles.container, { 'show-on-hover': !menuOpen })}
style={{ top: `${offset}px` }}
data-testid={selectors.container}
>
{dragClass && ( {dragClass && (
<div <div
className={cx(styles.square, styles.draggable, dragClass)} className={cx(styles.square, styles.draggable, dragClass)}
@ -62,7 +56,6 @@ export function HoverWidget({ menu, title, dragClass, children, offset = -32, on
title={title} title={title}
placement="bottom" placement="bottom"
menuButtonClass={styles.menuButton} menuButtonClass={styles.menuButton}
onVisibleChange={setMenuOpen}
onOpenMenu={onOpenMenu} onOpenMenu={onOpenMenu}
/> />
)} )}
@ -72,10 +65,6 @@ export function HoverWidget({ menu, title, dragClass, children, offset = -32, on
function getStyles(theme: GrafanaTheme2) { function getStyles(theme: GrafanaTheme2) {
return { return {
hidden: css({
visibility: 'hidden',
opacity: '0',
}),
container: css({ container: css({
label: 'hover-container-widget', label: 'hover-container-widget',
transition: `all .1s linear`, transition: `all .1s linear`,

View File

@ -5,6 +5,7 @@ import { GrafanaTheme2, LoadingState } from '@grafana/data';
import { selectors } from '@grafana/e2e-selectors'; import { selectors } from '@grafana/e2e-selectors';
import { useStyles2, useTheme2 } from '../../themes'; import { useStyles2, useTheme2 } from '../../themes';
import { getFocusStyles } from '../../themes/mixins';
import { DelayRender } from '../../utils/DelayRender'; import { DelayRender } from '../../utils/DelayRender';
import { Icon } from '../Icon/Icon'; import { Icon } from '../Icon/Icon';
import { LoadingBar } from '../LoadingBar/LoadingBar'; import { LoadingBar } from '../LoadingBar/LoadingBar';
@ -157,7 +158,9 @@ export function PanelChrome({
); );
return ( return (
<div className={styles.container} style={containerStyles} data-testid={testid}> // tabIndex={0} is needed for keyboard accessibility in the plot area
// eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex
<div className={styles.container} style={containerStyles} data-testid={testid} tabIndex={0}>
<div className={styles.loadingBarContainer}> <div className={styles.loadingBarContainer}>
{loadingState === LoadingState.Loading ? <LoadingBar width={width} ariaLabel="Panel loading bar" /> : null} {loadingState === LoadingState.Loading ? <LoadingBar width={width} ariaLabel="Panel loading bar" /> : null}
</div> </div>
@ -262,20 +265,21 @@ const getStyles = (theme: GrafanaTheme2) => {
'.show-on-hover': { '.show-on-hover': {
opacity: '0', opacity: '0',
visibility: 'hidden',
}, },
'&:focus-visible, &:hover': { '&:focus-visible, &:hover': {
// only show menu icon on hover or focused panel // only show menu icon on hover or focused panel
'.show-on-hover': { '.show-on-hover': {
opacity: '1', opacity: '1',
visibility: 'visible',
}, },
}, },
'&:focus-visible': { '&:focus-visible': getFocusStyles(theme),
outline: `1px solid ${theme.colors.action.focus}`,
},
'&:focus-within': { // The not:(:focus) clause is so that this rule is only applied when decendants are focused (important otherwise the hover header is visible when panel is clicked).
'&:focus-within:not(:focus)': {
'.show-on-hover': { '.show-on-hover': {
visibility: 'visible', visibility: 'visible',
opacity: '1', opacity: '1',

View File

@ -14,7 +14,6 @@ interface PanelMenuProps {
title?: string; title?: string;
placement?: TooltipPlacement; placement?: TooltipPlacement;
offset?: [number, number]; offset?: [number, number];
onVisibleChange?: (state: boolean) => void;
onOpenMenu?: () => void; onOpenMenu?: () => void;
} }
@ -25,7 +24,6 @@ export function PanelMenu({
offset, offset,
dragClassCancel, dragClassCancel,
menuButtonClass, menuButtonClass,
onVisibleChange,
onOpenMenu, onOpenMenu,
}: PanelMenuProps) { }: PanelMenuProps) {
const testId = title ? selectors.components.Panels.Panel.menu(title) : `panel-menu-button`; const testId = title ? selectors.components.Panels.Panel.menu(title) : `panel-menu-button`;
@ -35,9 +33,8 @@ export function PanelMenu({
if (show && onOpenMenu) { if (show && onOpenMenu) {
onOpenMenu(); onOpenMenu();
} }
return onVisibleChange;
}, },
[onOpenMenu, onVisibleChange] [onOpenMenu]
); );
return ( return (

View File

@ -42,9 +42,7 @@ export const VizLayout: VizLayoutComponentType = ({ width, height, legend, child
if (!legend) { if (!legend) {
return ( return (
<> <>
{/* tabIndex={0} is needed for keyboard accessibility in the plot area */} <div style={containerStyle} className={styles.viz}>
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */}
<div tabIndex={0} style={containerStyle} className={styles.viz}>
{children(width, height)} {children(width, height)}
</div> </div>
</> </>
@ -97,11 +95,7 @@ export const VizLayout: VizLayoutComponentType = ({ width, height, legend, child
return ( return (
<div style={containerStyle}> <div style={containerStyle}>
{/* tabIndex={0} is needed for keyboard accessibility in the plot area */} <div className={styles.viz}>{size && children(size.width, size.height)}</div>
{/* eslint-disable-next-line jsx-a11y/no-noninteractive-tabindex */}
<div tabIndex={0} className={styles.viz}>
{size && children(size.width, size.height)}
</div>
<div style={legendStyle} ref={legendRef}> <div style={legendStyle} ref={legendRef}>
<CustomScrollbar hideHorizontalTrack>{legend}</CustomScrollbar> <CustomScrollbar hideHorizontalTrack>{legend}</CustomScrollbar>
</div> </div>

View File

@ -13,12 +13,12 @@ const SHIFT_MULTIPLIER = 2 as const;
const KNOWN_KEYS = new Set(['ArrowRight', 'ArrowLeft', 'ArrowUp', 'ArrowDown', 'Shift', ' ']); const KNOWN_KEYS = new Set(['ArrowRight', 'ArrowLeft', 'ArrowUp', 'ArrowDown', 'Shift', ' ']);
const initHook = (u: uPlot) => { const initHook = (u: uPlot) => {
let vizLayoutViz: HTMLElement | null = u.root.closest('[tabindex]'); let parentWithFocus: HTMLElement | null = u.root.closest('[tabindex]');
let pressedKeys = new Set<string>(); let pressedKeys = new Set<string>();
let dragStartX: number | null = null; let dragStartX: number | null = null;
let keysLastHandledAt: number | null = null; let keysLastHandledAt: number | null = null;
if (!vizLayoutViz) { if (!parentWithFocus) {
return; return;
} }
@ -133,7 +133,7 @@ const initHook = (u: uPlot) => {
const onFocus = () => { const onFocus = () => {
// We only want to initialize the cursor if the user is using keyboard controls // We only want to initialize the cursor if the user is using keyboard controls
if (!vizLayoutViz?.matches(':focus-visible')) { if (!parentWithFocus?.matches(':focus-visible')) {
return; return;
} }
@ -150,18 +150,17 @@ const initHook = (u: uPlot) => {
u.setSelect({ left: 0, top: 0, width: 0, height: 0 }, false); u.setSelect({ left: 0, top: 0, width: 0, height: 0 }, false);
}; };
vizLayoutViz.addEventListener('keydown', onKeyDown); parentWithFocus.addEventListener('keydown', onKeyDown);
vizLayoutViz.addEventListener('keyup', onKeyUp); parentWithFocus.addEventListener('keyup', onKeyUp);
vizLayoutViz.addEventListener('focus', onFocus); parentWithFocus.addEventListener('focus', onFocus);
vizLayoutViz.addEventListener('blur', onBlur); parentWithFocus.addEventListener('blur', onBlur);
const onDestroy = () => { const onDestroy = () => {
vizLayoutViz?.removeEventListener('keydown', onKeyDown); parentWithFocus?.removeEventListener('keydown', onKeyDown);
vizLayoutViz?.removeEventListener('keyup', onKeyUp); parentWithFocus?.removeEventListener('keyup', onKeyUp);
vizLayoutViz?.removeEventListener('focus', onFocus); parentWithFocus?.removeEventListener('focus', onFocus);
vizLayoutViz?.removeEventListener('blur', onBlur); parentWithFocus?.removeEventListener('blur', onBlur);
parentWithFocus = null;
vizLayoutViz = null;
}; };
(u.hooks.destroy ??= []).push(onDestroy); (u.hooks.destroy ??= []).push(onDestroy);

View File

@ -57,6 +57,7 @@ export const TooltipPlugin = ({
const [coords, setCoords] = useState<CartesianCoords2D | null>(null); const [coords, setCoords] = useState<CartesianCoords2D | null>(null);
const [isActive, setIsActive] = useState<boolean>(false); const [isActive, setIsActive] = useState<boolean>(false);
const isMounted = useMountedState(); const isMounted = useMountedState();
let parentWithFocus: HTMLElement | null = null;
const pluginId = `TooltipPlugin`; const pluginId = `TooltipPlugin`;
@ -92,12 +93,16 @@ export const TooltipPlugin = ({
config.addHook('init', (u) => { config.addHook('init', (u) => {
plotInstance.current = u; plotInstance.current = u;
u.root.parentElement?.addEventListener('focus', plotEnter);
u.over.addEventListener('mouseenter', plotEnter); u.over.addEventListener('mouseenter', plotEnter);
u.root.parentElement?.addEventListener('blur', plotLeave);
u.over.addEventListener('mouseleave', plotLeave); u.over.addEventListener('mouseleave', plotLeave);
parentWithFocus = u.root.closest('[tabindex]');
if (parentWithFocus) {
parentWithFocus.addEventListener('focus', plotEnter);
parentWithFocus.addEventListener('blur', plotLeave);
}
if (sync && sync() === DashboardCursorSync.Crosshair) { if (sync && sync() === DashboardCursorSync.Crosshair) {
u.root.classList.add('shared-crosshair'); u.root.classList.add('shared-crosshair');
} }
@ -162,11 +167,15 @@ export const TooltipPlugin = ({
return () => { return () => {
setCoords(null); setCoords(null);
if (plotInstance.current) { if (plotInstance.current) {
plotInstance.current.over.removeEventListener('mouseleave', plotLeave); plotInstance.current.over.removeEventListener('mouseleave', plotLeave);
plotInstance.current.over.removeEventListener('mouseenter', plotEnter); plotInstance.current.over.removeEventListener('mouseenter', plotEnter);
plotInstance.current.root.parentElement?.removeEventListener('focus', plotEnter);
plotInstance.current.root.parentElement?.removeEventListener('blur', plotLeave); if (parentWithFocus) {
parentWithFocus.removeEventListener('focus', plotEnter);
parentWithFocus.removeEventListener('blur', plotLeave);
}
} }
}; };
}, [config, setCoords, setIsActive, setFocusedPointIdx, setFocusedPointIdxs]); }, [config, setCoords, setIsActive, setFocusedPointIdx, setFocusedPointIdxs]);

View File

@ -93,6 +93,7 @@ export class DashboardGrid extends PureComponent<Props> {
} }
this.props.dashboard.sortPanelsByGridPos(); this.props.dashboard.sortPanelsByGridPos();
this.forceUpdate();
}; };
triggerForceUpdate = () => { triggerForceUpdate = () => {