PanelChrome: Improve error state design (#63776)

* PanelChrome: Improve error state design

* Simplify logic for hoverHeader

* padding for the title only for the title, not the whole header

* Review fixes

* missed saving file

---------

Co-authored-by: polinaboneva <polina.boneva@grafana.com>
This commit is contained in:
Torkel Ödegaard
2023-03-09 10:06:25 +01:00
committed by GitHub
parent 4ee389676e
commit fb55dd5df6
6 changed files with 40 additions and 65 deletions

View File

@@ -76,6 +76,7 @@ function getStyles(theme: GrafanaTheme2) {
display: 'flex',
position: 'absolute',
zIndex: 1,
right: 0,
boxSizing: 'border-box',
alignItems: 'center',
background: theme.colors.background.secondary,

View File

@@ -60,11 +60,6 @@ it('renders panel with a header if prop leftItems', () => {
expect(screen.getByTestId('header-container')).toBeInTheDocument();
});
it('renders panel with hover header if no title, no leftItems, hoverHeader is undefined but menu is present', () => {
setup({ title: '', leftItems: undefined, hoverHeader: undefined, menu: <div>Menu</div> });
expect(screen.getByTestId('hover-header-container')).toBeInTheDocument();
});
it('renders panel with a hovering header if prop hoverHeader is true', () => {
setup({ title: 'Test Panel Header', hoverHeader: true });
@@ -87,12 +82,6 @@ it('renders panel with a header with icons in place if prop titleItems', () => {
expect(screen.getByTestId('title-items-container')).toBeInTheDocument();
});
it('renders panel with a hover header if prop menu is present and hoverHeader is false', () => {
setup({ menu: <div> Menu </div>, hoverHeader: false });
expect(screen.getByTestId('hover-header-container')).toBeInTheDocument();
});
it('renders panel with a show-on-hover menu icon if prop menu', () => {
setup({ menu: <div> Menu </div> });

View File

@@ -84,19 +84,7 @@ export function PanelChrome({
}: PanelChromeProps) {
const theme = useTheme2();
const styles = useStyles2(getStyles);
// To Do rely on hoverHeader prop for header, not separate props
// once hoverHeader is implemented
//
// Backwards compatibility for having a designated space for the header
const hasHeader =
hoverHeader === false &&
(title.length > 0 ||
titleItems !== undefined ||
description !== '' ||
loadingState === LoadingState.Streaming ||
(leftItems?.length ?? 0) > 0);
const hasHeader = !hoverHeader;
const headerHeight = getHeaderHeight(theme, hasHeader);
const { contentStyle, innerWidth, innerHeight } = getContentStyle(padding, theme, width, headerHeight, height);
@@ -122,13 +110,10 @@ export function PanelChrome({
</h6>
)}
<PanelDescription description={description} className={dragClassCancel} />
{titleItems !== undefined && (
<div className={cx(styles.titleItems, dragClassCancel)} data-testid="title-items-container">
{titleItems}
</div>
)}
<div className={cx(styles.titleItems, dragClassCancel)} data-testid="title-items-container">
<PanelDescription description={description} className={dragClassCancel} />
{titleItems}
</div>
{loadingState === LoadingState.Streaming && (
<Tooltip content="Streaming">
@@ -141,23 +126,34 @@ export function PanelChrome({
);
return (
<div
className={cx(styles.container, { [styles.regularHeader]: hasHeader })}
style={containerStyles}
aria-label={ariaLabel}
>
<div className={styles.container} style={containerStyles} aria-label={ariaLabel}>
<div className={styles.loadingBarContainer}>
{loadingState === LoadingState.Loading ? <LoadingBar width={width} ariaLabel="Panel loading bar" /> : null}
</div>
{(hoverHeader || !hasHeader) && menu && (
<HoverWidget menu={menu} title={title} offset={hoverHeaderOffset} dragClass={dragClass}>
{headerContent}
</HoverWidget>
{hoverHeader && (
<>
{menu && (
<HoverWidget menu={menu} title={title} offset={hoverHeaderOffset} dragClass={dragClass}>
{headerContent}
</HoverWidget>
)}
{statusMessage && (
<div className={styles.errorContainerFloating}>
<PanelStatus message={statusMessage} onClick={statusMessageOnClick} ariaLabel="Panel status" />
</div>
)}
</>
)}
{hasHeader && (
<div className={cx(styles.headerContainer, dragClass)} style={headerStyles} data-testid="header-container">
{statusMessage && (
<div className={dragClassCancel}>
<PanelStatus message={statusMessage} onClick={statusMessageOnClick} ariaLabel="Panel status" />
</div>
)}
{headerContent}
<div className={styles.rightAligned}>
@@ -175,15 +171,6 @@ export function PanelChrome({
</div>
)}
{statusMessage && (
<PanelStatus
className={cx(styles.errorContainer, dragClassCancel)}
message={statusMessage}
onClick={statusMessageOnClick}
ariaLabel="Panel status"
/>
)}
<div className={styles.content} style={contentStyle}>
{children(innerWidth, innerHeight)}
</div>
@@ -245,6 +232,7 @@ const getStyles = (theme: GrafanaTheme2) => {
visibility: 'hidden',
opacity: '0',
},
'&:focus-visible, &:hover': {
// only show menu icon on hover or focused panel
'.show-on-hover': {
@@ -256,8 +244,7 @@ const getStyles = (theme: GrafanaTheme2) => {
'&:focus-visible': {
outline: `1px solid ${theme.colors.action.focus}`,
},
}),
regularHeader: css({
'&:focus-within': {
'.show-on-hover': {
visibility: 'visible',
@@ -281,7 +268,6 @@ const getStyles = (theme: GrafanaTheme2) => {
label: 'panel-header',
display: 'flex',
alignItems: 'center',
padding: theme.spacing(0, 0, 0, padding),
}),
streaming: css({
label: 'panel-streaming',
@@ -295,7 +281,7 @@ const getStyles = (theme: GrafanaTheme2) => {
title: css({
label: 'panel-title',
marginBottom: 0, // override default h6 margin-bottom
paddingRight: theme.spacing(1),
padding: theme.spacing(0, padding),
textOverflow: 'ellipsis',
overflow: 'hidden',
whiteSpace: 'nowrap',
@@ -316,14 +302,11 @@ const getStyles = (theme: GrafanaTheme2) => {
visibility: 'hidden',
border: 'none',
}),
errorContainer: css({
errorContainerFloating: css({
label: 'error-container',
position: 'absolute',
left: '50%',
transform: 'translateX(-50%)',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
left: 0,
top: 0,
zIndex: theme.zIndex.tooltip,
}),
leftItems: css({

View File

@@ -1,4 +1,4 @@
import { cx, css } from '@emotion/css';
import { css } from '@emotion/css';
import React from 'react';
import { GrafanaTheme2 } from '@grafana/data';
@@ -7,18 +7,17 @@ import { useStyles2 } from '../../themes';
import { ToolbarButton } from '../ToolbarButton/ToolbarButton';
export interface Props {
className?: string;
message?: string;
onClick?: (e: React.SyntheticEvent) => void;
ariaLabel?: string;
}
export function PanelStatus({ className, message, onClick, ariaLabel = 'status' }: Props) {
export function PanelStatus({ message, onClick, ariaLabel = 'status' }: Props) {
const styles = useStyles2(getStyles);
return (
<ToolbarButton
className={cx(className, styles.buttonStyles)}
className={styles.buttonStyles}
onClick={onClick}
variant={'destructive'}
icon="exclamation-triangle"

View File

@@ -73,6 +73,7 @@ const getStyles = (theme: GrafanaTheme2) => {
}),
timeshift: css({
color: theme.colors.text.link,
gap: theme.spacing(0.5),
'&:hover': {
color: theme.colors.emphasize(theme.colors.text.link, 0.03),

View File

@@ -631,12 +631,13 @@ export class PanelStateWrapper extends PureComponent<Props, State> {
const { transparent } = panel;
const alertState = data.alertState?.state;
const hasHoverHeader = this.hasOverlayHeader();
const containerClassNames = classNames({
'panel-container': true,
'panel-container--absolute': isSoloRoute(locationService.getLocation().pathname),
'panel-container--transparent': transparent,
'panel-container--no-title': this.hasOverlayHeader(),
'panel-container--no-title': hasHoverHeader,
[`panel-alert-state--${alertState}`]: alertState !== undefined,
});
@@ -648,6 +649,7 @@ export class PanelStateWrapper extends PureComponent<Props, State> {
(data.series.length > 0 && data.series.some((v) => (v.meta?.notices?.length ?? 0) > 0)) ||
(data.request && data.request.timeInfo) ||
alertState;
const titleItems = showTitleItems && (
<PanelHeaderTitleItems
key="title-items"
@@ -685,7 +687,7 @@ export class PanelStateWrapper extends PureComponent<Props, State> {
dragClassCancel="grid-drag-cancel"
padding={padding}
hoverHeaderOffset={hoverHeaderOffset}
hoverHeader={title ? false : true}
hoverHeader={this.hasOverlayHeader()}
displayMode={transparent ? 'transparent' : 'default'}
>
{(innerWidth, innerHeight) => (