fix: address review findings for list numbering format UI

- Fix correctness bug: ranged selection now groups paragraphs by numId
  so separate lists stay separate instead of merging into one
- Extract shared isOrderedListParagraph() and applyFormatToGroup() helpers
- Remove duplicate setNumberedListIcon in defaultItems.js, use canonical
  superToolbar.updateNumberedListIcon()
- Simplify numbering-icon-map.js to direct lookup with nullish coalescing
- Simplify toggleList.js branching for format config

Author: caio-pizzol

packages/super-editor/src/components/toolbar/defaultItems.js

@@ -681,7 +681,7 @@ export const makeDefaultItems = ({
             // Store the selected type for future list creation
             superToolbar.selectedNumberingType = numberingType;
             // Update the numbered list button icon
-            setNumberedListIcon(numberedList, numberingType);
+            superToolbar.updateNumberedListIcon(numberedList, numberingType);
             // If we're in an active list, change its type
             const buttonWithCommand = { ...numberedListType, command: 'changeListNumberingType' };
             superToolbar.emitCommand({ item: buttonWithCommand, argument: numberingType });
@@ -699,23 +699,12 @@ export const makeDefaultItems = ({
     ],
     onActivate: ({ numberingType }) => {
       if (numberingType) {
-        // Update stored type when activating on an existing list
         superToolbar.selectedNumberingType = numberingType;
-        setNumberedListIcon(numberedList, numberingType);
+        superToolbar.updateNumberedListIcon(numberedList, numberingType);
       }
     },
-    onDeactivate: () => {
-      // Keep the selected type even when deactivated
-    },
   });
 
-  const setNumberedListIcon = (button, numberingType) => {
-    const icon = getNumberingIcon(numberingType);
-    if (button.icon && button.icon.value !== icon) {
-      button.icon.value = icon;
-    }
-  };
-
   // indent left
   const indentLeft = useToolbarItem({
     type: 'button',

packages/super-editor/src/components/toolbar/numbering-icon-map.js

@@ -3,21 +3,9 @@ import { numberingIcons } from './numbering-icons.js';
 
 /**
  * Get the icon for a given numbering format
- * Maps numbering format keys to their corresponding icons
  * @param {string} numberingType - The numbering format key
  * @returns {string} The SVG icon for the format
  */
 export function getNumberingIcon(numberingType) {
-  const iconMap = {
-    decimalPlain: numberingIcons.decimalPlain,
-    decimal: numberingIcons.decimal,
-    decimalParen: numberingIcons.decimalParen,
-    upperLetter: numberingIcons.upperLetter,
-    lowerLetter: numberingIcons.lowerLetter,
-    letterParen: numberingIcons.letterParen,
-    upperRoman: numberingIcons.upperRoman,
-    lowerRoman: numberingIcons.lowerRoman,
-  };
-
-  return iconMap[numberingType] || numberingIcons.decimal;
+  return numberingIcons[numberingType] ?? numberingIcons.decimal;
 }

packages/super-editor/src/core/commands/changeListNumberingType.js

@@ -4,6 +4,20 @@ import { ListHelpers } from '@helpers/list-numbering-helpers.js';
 import { updateNumberingProperties } from './changeListLevel.js';
 import { getFormatConfig } from '@helpers/numbering-format-config.js';
 
+/**
+ * Check if a paragraph node is an ordered list item
+ * @param {import('prosemirror-model').Node} node
+ * @param {object} paraProps - Resolved paragraph properties
+ * @returns {boolean}
+ */
+function isOrderedListParagraph(node, paraProps) {
+  return !!(
+    paraProps.numberingProperties &&
+    node.attrs.listRendering &&
+    node.attrs.listRendering.numberingType !== 'bullet'
+  );
+}
+
 /**
  * Find all adjacent paragraphs that share the same numbering properties (numId and ilvl)
  * @param {import('prosemirror-model').Node} doc - The document
@@ -15,31 +29,22 @@ import { getFormatConfig } from '@helpers/numbering-format-config.js';
 function findAdjacentListItems(doc, startPos, targetNumId, targetIlvl) {
   const matchingParagraphs = [];
 
-  // Helper to check if a paragraph matches our criteria
   const matchesTarget = (node) => {
     if (node.type.name !== 'paragraph') return false;
-
     const paraProps = getResolvedParagraphProperties(node);
-    const isOrderedList =
-      paraProps.numberingProperties && node.attrs.listRendering && node.attrs.listRendering.numberingType !== 'bullet';
-
-    if (!isOrderedList) return false;
-
+    if (!isOrderedListParagraph(node, paraProps)) return false;
     const numId = paraProps.numberingProperties?.numId;
     const ilvl = paraProps.numberingProperties?.ilvl ?? 0;
-
     return numId === targetNumId && ilvl === targetIlvl;
   };
 
-  // Collect all paragraphs in order, tracking their positions
   const allParagraphs = [];
   doc.descendants((node, pos) => {
     if (node.type.name === 'paragraph') {
       allParagraphs.push({ node, pos, paraProps: getResolvedParagraphProperties(node) });
     }
   });
 
-  // Find the index of the paragraph at startPos
   let startIndex = -1;
   for (let i = 0; i < allParagraphs.length; i++) {
     if (allParagraphs[i].pos === startPos) {
@@ -52,30 +57,55 @@ function findAdjacentListItems(doc, startPos, targetNumId, targetIlvl) {
     return matchingParagraphs;
   }
 
-  // Add the starting paragraph
   matchingParagraphs.push(allParagraphs[startIndex]);
 
-  // Search backwards for consecutive matching paragraphs
   for (let i = startIndex - 1; i >= 0; i--) {
     if (matchesTarget(allParagraphs[i].node)) {
       matchingParagraphs.unshift(allParagraphs[i]);
     } else {
-      break; // Stop when we hit a non-matching paragraph
+      break;
     }
   }
 
-  // Search forwards for consecutive matching paragraphs
   for (let i = startIndex + 1; i < allParagraphs.length; i++) {
     if (matchesTarget(allParagraphs[i].node)) {
       matchingParagraphs.push(allParagraphs[i]);
     } else {
-      break; // Stop when we hit a non-matching paragraph
+      break;
     }
   }
 
   return matchingParagraphs;
 }
 
+/**
+ * Apply a numbering format change to a group of paragraphs sharing the same numId
+ * @param {Array<{node: any, pos: number, paraProps: any}>} paragraphs
+ * @param {object} formatConfig - { fmt, lvlText }
+ * @param {object} editor
+ * @param {object} tr - ProseMirror transaction
+ */
+function applyFormatToGroup(paragraphs, formatConfig, editor, tr) {
+  const firstItem = paragraphs[0];
+  const existingIlvl = firstItem.paraProps.numberingProperties?.ilvl ?? 0;
+
+  const newNumId = ListHelpers.getNewListId(editor);
+  ListHelpers.generateNewListDefinition({
+    numId: Number(newNumId),
+    listType: 'orderedList',
+    level: String(existingIlvl),
+    start: '1',
+    text: formatConfig.lvlText,
+    fmt: formatConfig.fmt,
+    editor,
+  });
+
+  for (const { node, pos, paraProps } of paragraphs) {
+    const currentIlvl = paraProps.numberingProperties?.ilvl ?? 0;
+    updateNumberingProperties({ numId: Number(newNumId), ilvl: currentIlvl }, node, pos, editor, tr);
+  }
+}
+
 /**
  * Change the numbering type of an ordered list
  * @param {string} numberingFormat - The format to apply (decimal, lowerRoman, upperRoman, lowerLetter, upperLetter, etc.)
@@ -87,55 +117,39 @@ export const changeListNumberingType =
     const { selection } = state;
     const { from, to } = selection;
 
-    // Collect all paragraphs in selection that are part of an ordered list
     let paragraphsInSelection = [];
-
-    // Check if selection is collapsed (cursor position)
     const isCollapsed = from === to;
 
     if (isCollapsed) {
-      // Find the paragraph at cursor position
-      let cursorParagraph = null;
       let cursorPos = null;
 
       state.doc.nodesBetween(from - 1, from + 1, (node, pos) => {
         if (node.type.name === 'paragraph' && pos <= from && from <= pos + node.nodeSize) {
           const paraProps = getResolvedParagraphProperties(node);
-          const isOrderedList =
-            paraProps.numberingProperties &&
-            node.attrs.listRendering &&
-            node.attrs.listRendering.numberingType !== 'bullet';
-
-          if (isOrderedList) {
-            cursorParagraph = { node, pos, paraProps };
+          if (isOrderedListParagraph(node, paraProps)) {
             cursorPos = pos;
           }
           return false;
         }
         return true;
       });
 
-      if (cursorParagraph) {
-        const targetNumId = cursorParagraph.paraProps.numberingProperties?.numId;
-        const targetIlvl = cursorParagraph.paraProps.numberingProperties?.ilvl ?? 0;
-
-        // Find all adjacent paragraphs with the same numbering properties
+      if (cursorPos != null) {
+        const $pos = state.doc.resolve(cursorPos);
+        const node = $pos.nodeAfter;
+        const paraProps = getResolvedParagraphProperties(node);
+        const targetNumId = paraProps.numberingProperties?.numId;
+        const targetIlvl = paraProps.numberingProperties?.ilvl ?? 0;
         paragraphsInSelection = findAdjacentListItems(state.doc, cursorPos, targetNumId, targetIlvl);
       }
     } else {
-      // Non-collapsed selection: collect paragraphs within selection
       state.doc.nodesBetween(from, to, (node, pos) => {
         if (node.type.name === 'paragraph') {
           const paraProps = getResolvedParagraphProperties(node);
-          const isOrderedList =
-            paraProps.numberingProperties &&
-            node.attrs.listRendering &&
-            node.attrs.listRendering.numberingType !== 'bullet';
-
-          if (isOrderedList) {
+          if (isOrderedListParagraph(node, paraProps)) {
             paragraphsInSelection.push({ node, pos, paraProps });
           }
-          return false; // don't descend into paragraph content
+          return false;
         }
         return true;
       });
@@ -145,42 +159,26 @@ export const changeListNumberingType =
       return false;
     }
 
-    // Get the numId from the first list item to determine if we need to create a new definition
-    const firstListItem = paragraphsInSelection[0];
-    const existingNumId = firstListItem.paraProps.numberingProperties?.numId;
-    const existingIlvl = firstListItem.paraProps.numberingProperties?.ilvl ?? 0;
-
-    if (!existingNumId) {
+    const formatConfig = getFormatConfig(numberingFormat);
+    if (!formatConfig) {
       return false;
     }
 
-    // Map numbering format to Word's numFmt and lvlText
-    const formatConfig = getFormatConfig(numberingFormat);
-    if (!formatConfig) {
+    // Group paragraphs by numId so separate lists stay separate
+    const groups = new Map();
+    for (const item of paragraphsInSelection) {
+      const numId = item.paraProps.numberingProperties?.numId;
+      if (!numId) continue;
+      if (!groups.has(numId)) groups.set(numId, []);
+      groups.get(numId).push(item);
+    }
+
+    if (groups.size === 0) {
       return false;
     }
 
-    // Create a new list definition with the new numbering format
-    const newNumId = ListHelpers.getNewListId(editor);
-    ListHelpers.generateNewListDefinition({
-      numId: Number(newNumId),
-      listType: 'orderedList',
-      level: String(existingIlvl),
-      start: '1',
-      text: formatConfig.lvlText,
-      fmt: formatConfig.fmt,
-      editor,
-    });
-
-    // Apply the new numbering properties to all selected list items
-    for (const { node, pos, paraProps } of paragraphsInSelection) {
-      const currentIlvl = paraProps.numberingProperties?.ilvl ?? 0;
-      const newNumberingProps = {
-        numId: Number(newNumId),
-        ilvl: currentIlvl,
-      };
-
-      updateNumberingProperties(newNumberingProps, node, pos, editor, tr);
+    for (const group of groups.values()) {
+      applyFormatToGroup(group, formatConfig, editor, tr);
     }
 
     if (dispatch) dispatch(tr);

packages/super-editor/src/core/commands/toggleList.js

@@ -93,23 +93,18 @@ export const toggleList =
 
     if (mode === 'create') {
       const numId = ListHelpers.getNewListId(editor);
+      const formatConfig = listType === 'orderedList' && numberingFormat ? getFormatConfig(numberingFormat) : null;
 
-      // Use the provided numbering format or default settings
-      if (listType === 'orderedList' && numberingFormat) {
-        const formatConfig = getFormatConfig(numberingFormat);
-        if (formatConfig) {
-          ListHelpers.generateNewListDefinition({
-            numId: Number(numId),
-            listType,
-            level: '0',
-            start: '1',
-            text: formatConfig.lvlText,
-            fmt: formatConfig.fmt,
-            editor,
-          });
-        } else {
-          ListHelpers.generateNewListDefinition({ numId: Number(numId), listType, editor });
-        }
+      if (formatConfig) {
+        ListHelpers.generateNewListDefinition({
+          numId: Number(numId),
+          listType,
+          level: '0',
+          start: '1',
+          text: formatConfig.lvlText,
+          fmt: formatConfig.fmt,
+          editor,
+        });
       } else {
         ListHelpers.generateNewListDefinition({ numId: Number(numId), listType, editor });
       }