MM-67158 - fix overlap in post actions menu (#35415)

* MM-67158 - fix overlap in post actions menu

* use useState callback ref, keep all 9 icons in wide mode, fix a11y test

* fix unit test

* adjust e2e to new layout

* code review fixes and layout mode constants

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Pablo Vélez
2026-03-24 06:15:27 -05:00
committed by GitHub
co-authored by Mattermost Build
parent 7d26b7f317
commit 43130d8085
5 changed files with 282 additions and 95 deletions
@@ -202,7 +202,10 @@ describe('Verify Accessibility Support in different input fields', () => {
cy.get('#FormattingControl_ul').should('be.focused').and('have.attr', 'aria-label', 'bulleted list').tab();
// * Verify if the focus is on the numbered list button
cy.get('#FormattingControl_ol').should('be.focused').and('have.attr', 'aria-label', 'numbered list').tab().tab().tab();
cy.get('#FormattingControl_ol').should('be.focused').and('have.attr', 'aria-label', 'numbered list');
// # Skip any additional controls (priority, AI rewrite, BOR) which vary by enterprise config
cy.get('#toggleFormattingBarButton').focus();
// * Verify if the focus is on the formatting options button
cy.get('#toggleFormattingBarButton').should('be.focused').and('have.attr', 'aria-label', 'formatting').tab();
@@ -240,32 +243,23 @@ describe('Verify Accessibility Support in different input fields', () => {
// * Verify if the focus is on the bold button
cy.get('#FormattingControl_bold').should('be.focused').and('have.attr', 'aria-label', 'bold').tab();
// * Verify if the focus is on the italic button
cy.get('#FormattingControl_italic').should('be.focused').and('have.attr', 'aria-label', 'italic').tab();
// # Tab through any remaining visible formatting controls before the overflow button.
// # The number of visible controls depends on the RHS width and additional controls present.
cy.get('#HiddenControlsButtonRHS_COMMENT').focus().click().tab();
// * Verify if the focus is on the strike through button
cy.get('#FormattingControl_strike').should('be.focused').and('have.attr', 'aria-label', 'strike through').tab();
// * Verify hidden controls are accessible via the overflow menu
cy.get('#FormattingControl_italic').should('exist').and('have.attr', 'aria-label', 'italic');
cy.get('#FormattingControl_strike').should('exist').and('have.attr', 'aria-label', 'strike through');
cy.get('#FormattingControl_heading').should('exist').and('have.attr', 'aria-label', 'heading');
cy.get('#FormattingControl_link').should('exist').and('have.attr', 'aria-label', 'link');
cy.get('#FormattingControl_code').should('exist').and('have.attr', 'aria-label', 'code');
cy.get('#FormattingControl_quote').should('exist').and('have.attr', 'aria-label', 'quote');
cy.get('#FormattingControl_ul').should('exist').and('have.attr', 'aria-label', 'bulleted list');
cy.get('#FormattingControl_ol').should('exist').and('have.attr', 'aria-label', 'numbered list');
// * Verify if the focus is on the hidden controls button
cy.get('#HiddenControlsButtonRHS_COMMENT').should('be.focused').and('have.attr', 'aria-label', 'show hidden formatting options').click().tab();
// * Verify if the focus is on the hidden heading button
cy.get('#FormattingControl_heading').should('be.focused').and('have.attr', 'aria-label', 'heading').tab();
// * Verify if the focus is on the hidden link button
cy.get('#FormattingControl_link').should('be.focused').and('have.attr', 'aria-label', 'link').tab();
// * Verify if the focus is on the hidden code button
cy.get('#FormattingControl_code').should('be.focused').and('have.attr', 'aria-label', 'code').tab();
// * Verify if the focus is on the hidden quote button
cy.get('#FormattingControl_quote').should('be.focused').and('have.attr', 'aria-label', 'quote').tab();
// * Verify if the focus is on the hidden bulleted list button
cy.get('#FormattingControl_ul').should('be.focused').and('have.attr', 'aria-label', 'bulleted list').tab();
// * Verify if the focus is on the hidden numbered list button
cy.get('#FormattingControl_ol').should('be.focused').and('have.attr', 'aria-label', 'numbered list').tab();
// # Close the overflow popover, skip additional controls (priority, BOR) which vary by enterprise config
cy.get('#HiddenControlsButtonRHS_COMMENT').focus().type('{esc}');
cy.get('#toggleFormattingBarButton').focus();
// * Verify if the focus is on the formatting options button
cy.get('#toggleFormattingBarButton').should('be.focused').and('have.attr', 'aria-label', 'formatting').tab();
@@ -12,7 +12,7 @@ import * as Hooks from './hooks';
jest.mock('./hooks');
const {splitFormattingBarControls} = jest.requireActual('./hooks');
const {LayoutModes, splitFormattingBarControls} = jest.requireActual('./hooks');
describe('FormattingBar', () => {
const baseProps = {
@@ -24,7 +24,7 @@ describe('FormattingBar', () => {
};
test('should render hidden formatting button when screen size is min', () => {
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({wideMode: 'min', ...splitFormattingBarControls('min')});
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Min, ...splitFormattingBarControls('min')});
renderWithContext(
<FormattingBar {...baseProps}/>,
@@ -34,7 +34,7 @@ describe('FormattingBar', () => {
});
test('should render hidden formatting button when screen size is narrow', () => {
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({wideMode: 'narrow', ...splitFormattingBarControls('narrow')});
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Narrow, ...splitFormattingBarControls('narrow')});
renderWithContext(
<FormattingBar {...baseProps}/>,
@@ -44,7 +44,7 @@ describe('FormattingBar', () => {
});
test('should render hidden formatting button when screen size is normal', () => {
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({wideMode: 'normal', ...splitFormattingBarControls('normal')});
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Normal, ...splitFormattingBarControls('normal')});
renderWithContext(
<FormattingBar {...baseProps}/>,
@@ -54,7 +54,7 @@ describe('FormattingBar', () => {
});
test('should not render hidden formatting button when screen size is wide', () => {
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({wideMode: 'wide', ...splitFormattingBarControls('wide')});
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Wide, ...splitFormattingBarControls('wide')});
renderWithContext(
<FormattingBar {...baseProps}/>,
@@ -64,7 +64,7 @@ describe('FormattingBar', () => {
});
test('MM-56705 should not submit form when clicking on hidden formatting button', async () => {
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({wideMode: 'narrow', ...splitFormattingBarControls('narrow')});
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Narrow, ...splitFormattingBarControls('narrow')});
const onSubmit = jest.fn();
@@ -83,7 +83,7 @@ describe('FormattingBar', () => {
});
test('should disable tooltip when hidden controls are shown', async () => {
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({wideMode: 'narrow', ...splitFormattingBarControls('narrow')});
jest.spyOn(Hooks, 'useFormattingBarControls').mockReturnValue({layoutMode: LayoutModes.Narrow, ...splitFormattingBarControls('narrow')});
const {container} = renderWithContext(
<FormattingBar {...baseProps}/>,
@@ -3,7 +3,7 @@
import {useFloating, offset, useClick, useDismiss, useInteractions} from '@floating-ui/react';
import classNames from 'classnames';
import React, {memo, useCallback, useEffect, useRef, useState} from 'react';
import React, {memo, useCallback, useEffect, useMemo, useState} from 'react';
import {useIntl} from 'react-intl';
import {CSSTransition} from 'react-transition-group';
import styled from 'styled-components';
@@ -15,7 +15,7 @@ import WithTooltip from 'components/with_tooltip';
import type {ApplyMarkdownOptions, MarkdownMode} from 'utils/markdown/apply_markdown';
import FormattingIcon, {IconContainer} from './formatting_icon';
import {useFormattingBarControls} from './hooks';
import {LayoutModes, useFormattingBarControls} from './hooks';
export const Separator = styled.div`
display: block;
@@ -144,8 +144,12 @@ const FormattingBar = (props: FormattingBarProps): JSX.Element => {
additionalControls,
} = props;
const [showHiddenControls, setShowHiddenControls] = useState(false);
const formattingBarRef = useRef<HTMLDivElement>(null);
const {controls, hiddenControls, wideMode} = useFormattingBarControls(formattingBarRef);
const additionalControlsCount = useMemo(() => {
return Array.isArray(additionalControls) ? additionalControls.filter(Boolean).length : 0;
}, [additionalControls]);
const {formattingBarRef, controls, hiddenControls, layoutMode} = useFormattingBarControls(additionalControlsCount, location);
const {formatMessage} = useIntl();
const HiddenControlsButtonAriaLabel = formatMessage({id: 'accessibility.button.hidden_controls_button', defaultMessage: 'show hidden formatting options'});
@@ -169,9 +173,9 @@ const FormattingBar = (props: FormattingBarProps): JSX.Element => {
useEffect(() => {
update?.();
}, [wideMode, update, showHiddenControls]);
}, [layoutMode, update, showHiddenControls]);
const hasHiddenControls = wideMode !== 'wide';
const hasHiddenControls = layoutMode !== LayoutModes.Wide;
/**
* wrapping this factory in useCallback prevents it from constantly getting a new
@@ -206,7 +210,7 @@ const FormattingBar = (props: FormattingBarProps): JSX.Element => {
}
}, [getCurrentSelection, getCurrentMessage, applyMarkdown, showHiddenControls, disableControls]);
const leftPosition = wideMode === 'min' ? (x ?? 0) + DEFAULT_MIN_MODE_X_COORD : x ?? 0;
const leftPosition = layoutMode === LayoutModes.Min ? (x ?? 0) + DEFAULT_MIN_MODE_X_COORD : x ?? 0;
const hiddenControlsContainerStyles: React.CSSProperties = {
position: strategy,
@@ -214,7 +218,7 @@ const FormattingBar = (props: FormattingBarProps): JSX.Element => {
left: leftPosition,
};
const showSeparators = wideMode === 'wide';
const showSeparators = layoutMode === LayoutModes.Wide;
return (
<FormattingBarContainer
@@ -0,0 +1,149 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import {LayoutModes, splitFormattingBarControls} from './hooks';
describe('splitFormattingBarControls', () => {
describe('wide mode — always shows all 9 icons regardless of additional controls', () => {
test('shows all 9 with no additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Wide, 0, false);
expect(controls).toHaveLength(9);
expect(controls).toEqual(['bold', 'italic', 'strike', 'heading', 'link', 'code', 'quote', 'ul', 'ol']);
expect(hiddenControls).toHaveLength(0);
});
test('shows all 9 with 1 additional control', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Wide, 1, false);
expect(controls).toHaveLength(9);
expect(hiddenControls).toHaveLength(0);
});
test('shows all 9 with 3 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Wide, 3, false);
expect(controls).toHaveLength(9);
expect(hiddenControls).toHaveLength(0);
});
});
describe('normal mode (base 5) — first additional control is free, each one after reduces by 1', () => {
test('shows 5 with no additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 0, false);
expect(controls).toHaveLength(5);
expect(controls).toEqual(['bold', 'italic', 'strike', 'heading', 'link']);
expect(hiddenControls).toHaveLength(4);
});
test('shows 5 with 1 additional control', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 1, false);
expect(controls).toHaveLength(5);
expect(hiddenControls).toHaveLength(4);
});
test('shows 4 with 2 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 2, false);
expect(controls).toHaveLength(4);
expect(controls).toEqual(['bold', 'italic', 'strike', 'heading']);
expect(hiddenControls).toHaveLength(5);
});
test('shows 3 with 3 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 3, false);
expect(controls).toHaveLength(3);
expect(controls).toEqual(['bold', 'italic', 'strike']);
expect(hiddenControls).toHaveLength(6);
});
test('shows 2 with 4 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 4, false);
expect(controls).toHaveLength(2);
expect(hiddenControls).toHaveLength(7);
});
});
describe('narrow mode (base 2) — first additional control is free, each one after reduces by 1', () => {
test('shows 2 with no additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Narrow, 0, false);
expect(controls).toHaveLength(2);
expect(controls).toEqual(['bold', 'italic']);
expect(hiddenControls).toHaveLength(7);
});
test('shows 2 with 1 additional control', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Narrow, 1, false);
expect(controls).toHaveLength(2);
expect(hiddenControls).toHaveLength(7);
});
test('shows 1 with 2 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Narrow, 2, false);
expect(controls).toHaveLength(1);
expect(controls).toEqual(['bold']);
expect(hiddenControls).toHaveLength(8);
});
test('shows 0 with 3 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Narrow, 3, false);
expect(controls).toHaveLength(0);
expect(hiddenControls).toHaveLength(9);
});
});
describe('min mode (base 1) — first additional control is free, each one after reduces by 1', () => {
test('shows 1 with no additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Min, 0, false);
expect(controls).toHaveLength(1);
expect(controls).toEqual(['bold']);
expect(hiddenControls).toHaveLength(8);
});
test('shows 0 with 2 additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Min, 2, false);
expect(controls).toHaveLength(0);
expect(hiddenControls).toHaveLength(9);
});
});
describe('RHS — each additional control reduces by 1 (tighter space, no free slot)', () => {
test('wide mode keeps all 9 regardless of additional controls', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Wide, 3, true);
expect(controls).toHaveLength(9);
expect(hiddenControls).toHaveLength(0);
});
test('normal with 1 additional shows 4', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 1, true);
expect(controls).toHaveLength(4);
expect(hiddenControls).toHaveLength(5);
});
test('normal with 2 additional shows 3', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 2, true);
expect(controls).toHaveLength(3);
expect(hiddenControls).toHaveLength(6);
});
test('normal with 3 additional shows 2', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Normal, 3, true);
expect(controls).toHaveLength(2);
expect(hiddenControls).toHaveLength(7);
});
test('narrow with 1 additional shows 1', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Narrow, 1, true);
expect(controls).toHaveLength(1);
expect(hiddenControls).toHaveLength(8);
});
test('narrow with 2 additional shows 0', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Narrow, 2, true);
expect(controls).toHaveLength(0);
expect(hiddenControls).toHaveLength(9);
});
test('min with 1 additional shows 0', () => {
const {controls, hiddenControls} = splitFormattingBarControls(LayoutModes.Min, 1, true);
expect(controls).toHaveLength(0);
expect(hiddenControls).toHaveLength(9);
});
});
});
@@ -3,66 +3,105 @@
import type {Instance} from '@popperjs/core';
import debounce from 'lodash/debounce';
import type React from 'react';
import {useEffect, useLayoutEffect, useMemo, useState} from 'react';
import type {MarkdownMode} from 'utils/markdown/apply_markdown';
type WideMode = 'wide' | 'normal' | 'narrow' | 'min';
export const LayoutModes = {
Wide: 'wide',
Normal: 'normal',
Narrow: 'narrow',
Min: 'min',
} as const;
type LayoutMode = typeof LayoutModes[keyof typeof LayoutModes];
type Threshold = [minWidth: number, mode: LayoutMode];
// Ordered descending — first match (width >= threshold) wins, fallback is 'min'.
const THRESHOLDS_CENTER: Threshold[] = [
[641, LayoutModes.Wide],
[424, LayoutModes.Normal],
[310, LayoutModes.Narrow],
];
const THRESHOLDS_RHS: Threshold[] = [
[521, LayoutModes.Wide],
[380, LayoutModes.Normal],
[280, LayoutModes.Narrow],
];
const DEBOUNCE_DELAY = 10;
function resolveLayoutMode(width: number, thresholds: Threshold[]): LayoutMode {
for (const [minWidth, mode] of thresholds) {
if (width >= minWidth) {
return mode;
}
}
return LayoutModes.Min;
}
function isRHSLocation(location: string): boolean {
return location.toLowerCase().includes('rhs');
}
const useResponsiveFormattingBar = (element: HTMLDivElement | null, isRHS: boolean): LayoutMode => {
const [layoutMode, setLayoutMode] = useState<LayoutMode>(LayoutModes.Wide);
const useResponsiveFormattingBar = (ref: React.RefObject<HTMLDivElement>): WideMode => {
const [wideMode, setWideMode] = useState<WideMode>('wide');
const handleResize = useMemo(() => debounce(() => {
if (ref.current?.clientWidth == null) {
if (!element) {
return;
}
if (ref.current.clientWidth > 640) {
setWideMode('wide');
}
if (ref.current.clientWidth >= 424 && ref.current.clientWidth <= 640) {
setWideMode('normal');
}
if (ref.current.clientWidth < 424) {
setWideMode('narrow');
}
if (ref.current.clientWidth < 310) {
setWideMode('min');
}
}, 10), [ref]);
const thresholds = isRHS ? THRESHOLDS_RHS : THRESHOLDS_CENTER;
setLayoutMode(resolveLayoutMode(element.clientWidth, thresholds));
}, DEBOUNCE_DELAY), [element, isRHS]);
useLayoutEffect(() => {
if (!ref.current) {
if (!element) {
return () => {};
}
let sizeObserver: ResizeObserver | null = new ResizeObserver(handleResize);
sizeObserver.observe(ref.current);
const sizeObserver = new ResizeObserver(handleResize);
sizeObserver.observe(element);
return () => {
sizeObserver!.disconnect();
sizeObserver = null;
handleResize.cancel();
sizeObserver.disconnect();
};
}, [handleResize, ref]);
}, [handleResize, element]);
return wideMode;
return layoutMode;
};
const MAP_WIDE_MODE_TO_CONTROLS_QUANTITY: {[key in WideMode]: number} = {
wide: 9,
normal: 5,
narrow: 3,
min: 1,
// Base icon counts for each mode (no additional controls)
const CONTROLS_COUNT_BASE: Record<LayoutMode, number> = {
[LayoutModes.Wide]: 9,
[LayoutModes.Normal]: 5,
[LayoutModes.Narrow]: 2,
[LayoutModes.Min]: 1,
};
export function splitFormattingBarControls(wideMode: WideMode) {
const allControls: MarkdownMode[] = ['bold', 'italic', 'strike', 'heading', 'link', 'code', 'quote', 'ul', 'ol'];
// All available formatting controls in priority order
const ALL_CONTROLS: MarkdownMode[] = ['bold', 'italic', 'strike', 'heading', 'link', 'code', 'quote', 'ul', 'ol'];
const controlsLength = MAP_WIDE_MODE_TO_CONTROLS_QUANTITY[wideMode];
// Wide layout always shows all icons — there is enough room regardless of additional controls.
// Center channel: reduction starts from the 2nd additional control (1 extra always fits).
// RHS: reduction starts from the 1st additional control (tighter space).
function getVisibleControlsCount(layoutMode: LayoutMode, additionalControlsCount: number, isRHS: boolean): number {
const base = CONTROLS_COUNT_BASE[layoutMode];
if (layoutMode === LayoutModes.Wide) {
return base;
}
const reduction = isRHS ? additionalControlsCount : Math.max(0, additionalControlsCount - 1);
return Math.max(0, base - reduction);
}
const controls = allControls.slice(0, controlsLength);
const hiddenControls = allControls.slice(controlsLength);
export function splitFormattingBarControls(layoutMode: LayoutMode, additionalControlsCount: number = 0, isRHS: boolean = false) {
const visibleControlsCount = getVisibleControlsCount(layoutMode, additionalControlsCount, isRHS);
const controls = ALL_CONTROLS.slice(0, visibleControlsCount);
const hiddenControls = ALL_CONTROLS.slice(visibleControlsCount);
return {
controls,
@@ -71,35 +110,36 @@ export function splitFormattingBarControls(wideMode: WideMode) {
}
export const useFormattingBarControls = (
formattingBarRef: React.RefObject<HTMLDivElement>,
additionalControlsCount: number = 0,
location: string = '',
): {
controls: MarkdownMode[];
hiddenControls: MarkdownMode[];
wideMode: WideMode;
} => {
const wideMode = useResponsiveFormattingBar(formattingBarRef);
formattingBarRef: (node: HTMLDivElement | null) => void;
controls: MarkdownMode[];
hiddenControls: MarkdownMode[];
layoutMode: LayoutMode;
} => {
const [element, setElement] = useState<HTMLDivElement | null>(null);
const {controls, hiddenControls} = splitFormattingBarControls(wideMode);
const isRHS = useMemo(() => isRHSLocation(location), [location]);
const layoutMode = useResponsiveFormattingBar(element, isRHS);
const {controls, hiddenControls} = useMemo(() => {
return splitFormattingBarControls(layoutMode, additionalControlsCount, isRHS);
}, [layoutMode, additionalControlsCount, isRHS]);
return {
formattingBarRef: setElement,
controls,
hiddenControls,
wideMode,
layoutMode,
};
};
export const useUpdateOnVisibilityChange = (update: Instance['update'] | null, isVisible: boolean) => {
const updateComponent = async () => {
if (!update) {
return;
}
await update();
};
useEffect(() => {
if (!isVisible) {
if (!isVisible || !update) {
return;
}
updateComponent();
}, [isVisible]);
update();
}, [isVisible, update]);
};