MM-51016 Re-implement layout logic for Markdown lists (#36678)

* Revert mattermost/mattermost-webapp#4981

This directly fixes the MM-51016, but it causes MM-22969 to regress on Firefox.

* Revert mattermost/mattermost-webapp#4844

This fixes the MM-22969, but it also causes MM-22192 to regress :p

* Re-implement layout logic for list items without using display:table

It turns out this is actually similar to my original implementation in
mattermost/mattermost-webapp#4328, but I changed it to a CSS-based solution during
reviews that turns out to have been much more janky.

* Change MarkdownListOrdered to set padding directly

* Fix bug in digit counting and add tests

* Improve handling of potential invalid start index

* Simplify that handling

* Add additional test case

* Improve test assertion
This commit is contained in:
Harrison Healey
2026-06-22 09:41:23 -04:00
committed by GitHub
parent eb76344ec7
commit 94e2efbd99
7 changed files with 112 additions and 26 deletions
@@ -114,4 +114,42 @@ describe('components/Markdown', () => {
expect(img).toHaveAttribute('src', expect.stringMatching(`/image\\?url=${encodeURIComponent(urlWithParams)}$`));
});
});
describe('ordered lists', () => {
test('should start lists at the correct number and set padding based on the final number', () => {
const {rerender} = renderWithContext(<Markdown {...baseProps}/>);
for (const [startIndex, endIndex, expectedPadding] of [
[0, 3, '2.5ch'],
[1, 9, '2.5ch'],
[5, 10, '3.5ch'],
[10, 99, '3.5ch'],
[50, 150, '4.5ch'],
[1, 1000, '5.5ch'],
[999999, 1000000, '8.5ch'],
] as const) {
let message = '';
for (let i = startIndex; i <= endIndex; i++) {
message += `${i}. item\n`;
}
rerender(
<Markdown
{...baseProps}
message={message}
/>,
);
const list = document.querySelector('ol.markdown__list');
expect(list).not.toBeNull();
expect(getComputedStyle(list!)).toMatchObject({
// This value starts at one less than what's displayed to users
'counter-reset': `list ${startIndex - 1}`,
'padding-left': expectedPadding,
});
}
});
});
});
@@ -0,0 +1,35 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
import React, {useMemo} from 'react';
export interface MarkdownListOrderedProps extends React.OlHTMLAttributes<HTMLOListElement> {}
export default function MarkdownListOrdered({children, start = 1, ...otherProps}: MarkdownListOrderedProps) {
const end = (start + React.Children.count(children)) - 1;
const style = useMemo<React.CSSProperties>(() => {
const digits = end.toString().length;
// Add 1.5 characters to account for the width of the decimal and two spaces. There's a chance that this
// doesn't work on all fonts, but it works at least with the default one
const markerWidth = `${digits + 1.5}ch`;
return {
counterReset: `list ${start - 1}`,
// Leave space for the item numbers
paddingLeft: markerWidth,
};
}, [end, start]);
return (
<ol
start={start}
style={style}
{...otherProps}
>
{children}
</ol>
);
}
@@ -1418,10 +1418,6 @@
}
li {
p {
display: inline;
}
ul {
padding: 0 0 0 20px;
margin: 0;
+16 -11
View File
@@ -354,28 +354,33 @@ h6.markdown__heading {
}
ol.markdown__list {
display: table;
padding: 0;
counter-reset: list;
// This element has padding-left and counter-reset set by the MarkdownListOrdered component
list-style-type: decimal;
>li {
display: table-row;
counter-increment: list;
word-break: break-word;
&::before {
display: table-cell;
padding-right: 8px;
content: counter(list) ".";
text-align: right;
word-break: keep-all;
&::marker {
/*
* The extra spaces are intentional to cause Chrome/Firefox to match the padding of Safari which doesn't
* support setting the content of the marker as of May 2026.
*
* Once it does, those extra spaces can be removed, a padding-left of 8px can be added to the li, and 1ch
* can be removed from --marker-width.
*/
content: counter(list) ". ";
}
// Special rules for task items within ordered lists
&.list-item--task-list {
&::before {
list-style-type: none;
&::marker {
// The counter is erased for task items
content: "";
}
input[type="checkbox"] {
// Margins are adjusted to align better the checkbox with the text
margin-right: 2px;
@@ -44,7 +44,7 @@ x^2 + y^2 = z^2
const input = `- a
- b
- c`;
const expected = '<ul className="markdown__list"><li><span>a</span></li><li><span>b</span></li><li><span>c</span></li></ul>';
const expected = '<ul class="markdown__list"><li>a</li><li>b</li><li>c</li></ul>';
const output = format(input);
expect(output).toBe(expected);
@@ -54,7 +54,7 @@ x^2 + y^2 = z^2
const input = `1. a
2. b
3. c`;
const expected = '<ol className="markdown__list" style="counter-reset: list 0"><li><span>a</span></li><li><span>b</span></li><li><span>c</span></li></ol>';
const expected = '<ol class="markdown__list" start="1"><li>a</li><li>b</li><li>c</li></ol>';
const output = format(input);
expect(output).toBe(expected);
@@ -64,7 +64,7 @@ x^2 + y^2 = z^2
const input = `0. a
1. b
2. c`;
const expected = '<ol className="markdown__list" style="counter-reset: list -1"><li><span>a</span></li><li><span>b</span></li><li><span>c</span></li></ol>';
const expected = '<ol class="markdown__list" start="0"><li>a</li><li>b</li><li>c</li></ol>';
const output = format(input);
expect(output).toBe(expected);
@@ -74,7 +74,7 @@ x^2 + y^2 = z^2
const input = `999. a
1. b
1. c`;
const expected = '<ol className="markdown__list" style="counter-reset: list 998"><li><span>a</span></li><li><span>b</span></li><li><span>c</span></li></ol>';
const expected = '<ol class="markdown__list" start="999"><li>a</li><li>b</li><li>c</li></ol>';
const output = format(input);
expect(output).toBe(expected);
@@ -226,11 +226,9 @@ export default class Renderer extends marked.Renderer {
public list(content: string, ordered: boolean, start: number) {
const type = ordered ? 'ol' : 'ul';
let output = `<${type} className="markdown__list"`;
let output = `<${type} class="markdown__list"`;
if (ordered && start !== undefined) {
// The CSS that we use for lists hides the actual counter and uses ::before to simulate one so that we can
// style it properly. We need to use a CSS counter to tell the ::before elements which numbers to show.
output += ` style="counter-reset: list ${start - 1}"`;
output += ` start="${start}"`;
}
output += `>${content}</${type}>`;
@@ -247,9 +245,7 @@ export default class Renderer extends marked.Renderer {
'/> '}${text.replace(taskListReg, '')}</li>`;
}
// Added a span because if not whitespace nodes only
// works in Firefox but not in Webkit
return `<li><span>${text}</span></li>`;
return `<li>${text}</li>`;
}
public text(txt: string) {
@@ -12,6 +12,7 @@ import InlineEntityLink from 'components/inline_entity_link';
import LatexBlock from 'components/latex_block';
import LatexInline from 'components/latex_inline';
import MarkdownImage from 'components/markdown_image';
import MarkdownListOrdered from 'components/markdown_list_ordered';
import PluginLinkTooltip from 'components/plugin_link_tooltip';
import PostEmoji from 'components/post_emoji';
import PostEditedIndicator from 'components/post_view/post_edited_indicator';
@@ -102,6 +103,21 @@ export default function messageHtmlToComponent(html: string, options: Options =
) : null;
},
},
{
replaceChildren: false,
shouldProcessNode: (node: any) => node.type === 'tag' && node.name === 'ol',
processNode: (node: any, children: React.ReactNode, index?: number) => {
return (
<MarkdownListOrdered
key={index}
className={node.attribs.class}
start={node.attribs.start ? parseInt(node.attribs.start, 10) : undefined}
>
{children}
</MarkdownListOrdered>
);
},
},
];
processingInstructions.push({