From 94e2efbd995c599f11362ec6a5260252b6433df1 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 22 Jun 2026 09:41:23 -0400 Subject: [PATCH] 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 --- .../src/components/markdown/markdown.test.tsx | 38 +++++++++++++++++++ .../src/components/markdown_list_ordered.tsx | 35 +++++++++++++++++ .../channels/src/sass/components/_post.scss | 4 -- .../channels/src/sass/layout/_markdown.scss | 27 +++++++------ .../channels/src/utils/markdown/index.test.ts | 8 ++-- .../channels/src/utils/markdown/renderer.tsx | 10 ++--- .../src/utils/message_html_to_component.tsx | 16 ++++++++ 7 files changed, 112 insertions(+), 26 deletions(-) create mode 100644 webapp/channels/src/components/markdown_list_ordered.tsx diff --git a/webapp/channels/src/components/markdown/markdown.test.tsx b/webapp/channels/src/components/markdown/markdown.test.tsx index 15dee3f478b..992450ebc1b 100644 --- a/webapp/channels/src/components/markdown/markdown.test.tsx +++ b/webapp/channels/src/components/markdown/markdown.test.tsx @@ -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(); + + 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( + , + ); + + 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, + }); + } + }); + }); }); diff --git a/webapp/channels/src/components/markdown_list_ordered.tsx b/webapp/channels/src/components/markdown_list_ordered.tsx new file mode 100644 index 00000000000..898e4074dd9 --- /dev/null +++ b/webapp/channels/src/components/markdown_list_ordered.tsx @@ -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 {} + +export default function MarkdownListOrdered({children, start = 1, ...otherProps}: MarkdownListOrderedProps) { + const end = (start + React.Children.count(children)) - 1; + + const style = useMemo(() => { + 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 ( +
    + {children} +
+ ); +} diff --git a/webapp/channels/src/sass/components/_post.scss b/webapp/channels/src/sass/components/_post.scss index c6500b2a4c1..854fc3e4f57 100644 --- a/webapp/channels/src/sass/components/_post.scss +++ b/webapp/channels/src/sass/components/_post.scss @@ -1418,10 +1418,6 @@ } li { - p { - display: inline; - } - ul { padding: 0 0 0 20px; margin: 0; diff --git a/webapp/channels/src/sass/layout/_markdown.scss b/webapp/channels/src/sass/layout/_markdown.scss index 43ed9837af5..a3223326224 100644 --- a/webapp/channels/src/sass/layout/_markdown.scss +++ b/webapp/channels/src/sass/layout/_markdown.scss @@ -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; diff --git a/webapp/channels/src/utils/markdown/index.test.ts b/webapp/channels/src/utils/markdown/index.test.ts index b40ecd979c8..0913b7202b2 100644 --- a/webapp/channels/src/utils/markdown/index.test.ts +++ b/webapp/channels/src/utils/markdown/index.test.ts @@ -44,7 +44,7 @@ x^2 + y^2 = z^2 const input = `- a - b - c`; - const expected = '
  • a
  • b
  • c
'; + const expected = '
  • a
  • b
  • c
'; 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 = '
  1. a
  2. b
  3. c
'; + const expected = '
  1. a
  2. b
  3. c
'; 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 = '
  1. a
  2. b
  3. c
'; + const expected = '
  1. a
  2. b
  3. c
'; 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 = '
  1. a
  2. b
  3. c
'; + const expected = '
  1. a
  2. b
  3. c
'; const output = format(input); expect(output).toBe(expected); diff --git a/webapp/channels/src/utils/markdown/renderer.tsx b/webapp/channels/src/utils/markdown/renderer.tsx index f3871f49d07..82d39bbda4e 100644 --- a/webapp/channels/src/utils/markdown/renderer.tsx +++ b/webapp/channels/src/utils/markdown/renderer.tsx @@ -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}`; @@ -247,9 +245,7 @@ export default class Renderer extends marked.Renderer { '/> '}${text.replace(taskListReg, '')}`; } - // Added a span because if not whitespace nodes only - // works in Firefox but not in Webkit - return `
  • ${text}
    • `; + return `
  • ${text}
    • `; } public text(txt: string) { diff --git a/webapp/channels/src/utils/message_html_to_component.tsx b/webapp/channels/src/utils/message_html_to_component.tsx index 078577890ae..33c1e9b51b1 100644 --- a/webapp/channels/src/utils/message_html_to_component.tsx +++ b/webapp/channels/src/utils/message_html_to_component.tsx @@ -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 ( + + {children} + + ); + }, + }, ]; processingInstructions.push({