feat: solve the problem of numbers changing after li tag indentation#469
feat: solve the problem of numbers changing after li tag indentation#469wuyiping0628 wants to merge 1 commit into
Conversation
WalkthroughThe PR updates CSS rules in the Quill editor stylesheet to manage ordered-list indentation markers. Rules for ChangesOrdered-list marker styling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/fluent-editor/src/assets/editor.scss (1)
128-258: ⚡ Quick winReduce code duplication using SCSS loops.
The nine rules for
li.ql-indent-{1-9} .ql-ui:before(lines 128-130, 144-146, 160-162, 176-178, 192-194, 208-210, 224-226, 240-242, 256-258) are nearly identical and could be generated using an SCSS@forloop to improve maintainability.♻️ Refactor using SCSS loop
- li.ql-indent-1 .ql-ui:before { - content: counter(list-1, decimal) '. ' !important; - } - li.ordered.ql-indent-1 { counter-reset: list-2 list-3 list-4 list-5 list-6 list-7 list-8 list-9; } li.ordered.ql-indent-2 { counter-increment: list-2; } li.ordered.ql-indent-2::before { content: counter(list-2, lower-roman) '. '; } - li.ql-indent-2 .ql-ui:before { - content: counter(list-2, decimal) '. ' !important; - } - li.ordered.ql-indent-2 { counter-reset: list-3 list-4 list-5 list-6 list-7 list-8 list-9; } - li.ordered.ql-indent-3 { - counter-increment: list-3; - } - - li.ordered.ql-indent-3::before { - content: counter(list-3, decimal) '. '; - } - - li.ql-indent-3 .ql-ui:before { - content: counter(list-3, decimal) '. ' !important; - } - - li.ordered.ql-indent-3 { - counter-reset: list-4 list-5 list-6 list-7 list-8 list-9; - } - - li.ordered.ql-indent-4 { - counter-increment: list-4; - } - - li.ordered.ql-indent-4::before { - content: counter(list-4, lower-alpha) '. '; - } - - li.ql-indent-4 .ql-ui:before { - content: counter(list-4, decimal) '. ' !important; - } - - li.ordered.ql-indent-4 { - counter-reset: list-5 list-6 list-7 list-8 list-9; - } - - li.ordered.ql-indent-5 { - counter-increment: list-5; - } - - li.ordered.ql-indent-5::before { - content: counter(list-5, lower-roman) '. '; - } - - li.ql-indent-5 .ql-ui:before { - content: counter(list-5, decimal) '. ' !important; - } - - li.ordered.ql-indent-5 { - counter-reset: list-6 list-7 list-8 list-9; - } - - li.ordered.ql-indent-6 { - counter-increment: list-6; - } - - li.ordered.ql-indent-6::before { - content: counter(list-6, decimal) '. '; - } - - li.ql-indent-6 .ql-ui:before { - content: counter(list-6, decimal) '. ' !important; - } - - li.ordered.ql-indent-6 { - counter-reset: list-7 list-8 list-9; - } - - li.ordered.ql-indent-7 { - counter-increment: list-7; - } - - li.ordered.ql-indent-7::before { - content: counter(list-7, lower-alpha) '. '; - } - - li.ql-indent-7 .ql-ui:before { - content: counter(list-7, decimal) '. ' !important; - } - - li.ordered.ql-indent-7 { - counter-reset: list-8 list-9; - } - - li.ordered.ql-indent-8 { - counter-increment: list-8; - } - - li.ordered.ql-indent-8::before { - content: counter(list-8, lower-roman) '. '; - } - - li.ql-indent-8 .ql-ui:before { - content: counter(list-8, decimal) '. ' !important; - } - - li.ordered.ql-indent-8 { - counter-reset: list-9; - } - - li.ordered.ql-indent-9 { - counter-increment: list-9; - } - - li.ordered.ql-indent-9::before { - content: counter(list-9, decimal) '. '; - } - - li.ql-indent-9 .ql-ui:before { - content: counter(list-9, decimal) '. ' !important; - } + // Generate .ql-ui:before rules for indent levels 1-9 + `@for` $i from 1 through 9 { + li.ql-indent-#{$i} .ql-ui:before { + content: counter(list-#{$i}, decimal) '. ' !important; + } + } + + // Keep existing counter-increment and counter-reset rules + `@for` $i from 3 through 9 { + li.ordered.ql-indent-#{$i} { + counter-increment: list-#{$i}; + } + + li.ordered.ql-indent-#{$i}::before { + `@if` $i == 3 or $i == 6 or $i == 9 { + content: counter(list-#{$i}, decimal) '. '; + } `@else` if $i == 4 or $i == 7 { + content: counter(list-#{$i}, lower-alpha) '. '; + } `@else` { + content: counter(list-#{$i}, lower-roman) '. '; + } + } + + li.ordered.ql-indent-#{$i} { + $reset-list: (); + `@for` $j from ($i + 1) through 9 { + $reset-list: append($reset-list, list-#{$j}, space); + } + counter-reset: $reset-list; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/fluent-editor/src/assets/editor.scss` around lines 128 - 258, There are nine near-identical rules for the selector pattern "li.ql-indent-{1..9} .ql-ui:before" that can be replaced by an SCSS loop; remove the duplicated explicit rules and add a single `@for` loop that iterates 1 through 9 and generates "li.ql-indent-#{$i} .ql-ui:before" with content using counter(list-#{$i}, decimal) '. ' !important so each indent level still references the corresponding list counter; update the stylesheet in packages/fluent-editor/src/assets/editor.scss by replacing the nine duplicated blocks with this loop to reduce duplication and improve maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/fluent-editor/src/assets/editor.scss`:
- Around line 128-130: The change forces all nested list counters to decimal
(e.g., li.ql-indent-1 .ql-ui:before) which removes the original varied styles
(lower-alpha, lower-roman) and harms visual hierarchy; restore the original
counter styles per indent level instead of using decimal for all indents
(reapply the previous counter(...) values for li.ql-indent-1, li.ql-indent-2,
li.ql-indent-4, li.ql-indent-5, li.ql-indent-7, li.ql-indent-8) or implement a
mapping from indent level to counter style and use that in the .ql-ui:before
rules, and apply the same fix to the other modified blocks mentioned (lines
around 144-146, 160-162, 176-178, 192-194, 208-210, 224-226, 240-242, 256-258)
so numbering styles preserve nesting distinctions.
- Around line 128-130: The new rules like "li.ql-indent-1 .ql-ui:before" are
causing dual markers and overbroad matching (affecting bullets) and overwrite
the original ordered styles ("li.ordered.ql-indent-1::before"); remove or
refactor those ".ql-ui:before" rules so they don't conflict: either delete the
nine "li.ql-indent-{1-9} .ql-ui:before" selectors if the existing
"li.ordered.ql-indent-{n}::before" rules already render correctly, or replace
them with a single SCSS mixin that only applies to ordered lists (guarded by the
".ordered" class) and preserves the alternating formats (lower-alpha,
lower-roman, decimal) instead of forcing decimal and using !important.
---
Nitpick comments:
In `@packages/fluent-editor/src/assets/editor.scss`:
- Around line 128-258: There are nine near-identical rules for the selector
pattern "li.ql-indent-{1..9} .ql-ui:before" that can be replaced by an SCSS
loop; remove the duplicated explicit rules and add a single `@for` loop that
iterates 1 through 9 and generates "li.ql-indent-#{$i} .ql-ui:before" with
content using counter(list-#{$i}, decimal) '. ' !important so each indent level
still references the corresponding list counter; update the stylesheet in
packages/fluent-editor/src/assets/editor.scss by replacing the nine duplicated
blocks with this loop to reduce duplication and improve maintainability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 812286df-c4f2-48ac-8a3a-274d3afe0595
📒 Files selected for processing (1)
packages/fluent-editor/src/assets/editor.scss
| li.ql-indent-1 .ql-ui:before { | ||
| content: counter(list-1, decimal) '. ' !important; | ||
| } |
There was a problem hiding this comment.
Consider the impact of forcing all indent levels to decimal numbering.
The new rules override the original varied counter styles with uniform decimal numbering:
- Original indent-1:
lower-alpha(a, b, c) → Now:decimal(1, 2, 3) - Original indent-2:
lower-roman(i, ii, iii) → Now:decimal(1, 2, 3) - Original indent-4:
lower-alpha→ Now:decimal - Original indent-5:
lower-roman→ Now:decimal - Original indent-7:
lower-alpha→ Now:decimal - Original indent-8:
lower-roman→ Now:decimal
While this change solves the stated problem of "numbers changing after indentation," it eliminates the visual hierarchy that different numbering styles provide in deeply nested lists. Users may find it harder to distinguish nesting levels when all levels use the same decimal format.
If the varied styles are intentionally being removed, consider documenting this behavior change and its UX implications. If the original varied styles should be preserved while fixing the indentation bug, the root cause may require a different solution.
Also applies to: 144-146, 160-162, 176-178, 192-194, 208-210, 224-226, 240-242, 256-258
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/fluent-editor/src/assets/editor.scss` around lines 128 - 130, The
change forces all nested list counters to decimal (e.g., li.ql-indent-1
.ql-ui:before) which removes the original varied styles (lower-alpha,
lower-roman) and harms visual hierarchy; restore the original counter styles per
indent level instead of using decimal for all indents (reapply the previous
counter(...) values for li.ql-indent-1, li.ql-indent-2, li.ql-indent-4,
li.ql-indent-5, li.ql-indent-7, li.ql-indent-8) or implement a mapping from
indent level to counter style and use that in the .ql-ui:before rules, and apply
the same fix to the other modified blocks mentioned (lines around 144-146,
160-162, 176-178, 192-194, 208-210, 224-226, 240-242, 256-258) so numbering
styles preserve nesting distinctions.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify when .ql-ui elements are used and whether they coexist with li::before markers
# Search for .ql-ui usage in JavaScript/TypeScript files
echo "=== Searching for .ql-ui element creation and usage ==="
rg -n -C3 --type=ts --type=js 'ql-ui' -g '!*.scss' -g '!*.css'
# Check if there's logic that conditionally uses .ql-ui vs li::before
echo -e "\n=== Checking for list rendering logic ==="
rg -n -C5 --type=ts --type=js 'ordered.*indent|indent.*ordered' -g '!*.scss'
# Look for bullet list handling with indent classes
echo -e "\n=== Checking bullet list + indent interaction ==="
rg -n -C3 --type=ts --type=js 'bullet.*ql-indent|ql-indent.*bullet'Repository: opentiny/tiny-editor
Length of output: 3138
🏁 Script executed:
cat -n packages/fluent-editor/src/assets/editor.scss | head -280Repository: opentiny/tiny-editor
Length of output: 7290
🏁 Script executed:
rg -n "li\.ordered\.ql-indent|li\.ql-indent" packages/fluent-editor/src/assets/editor.scssRepository: opentiny/tiny-editor
Length of output: 1345
Remove or refactor the new li.ql-indent-{1-9} .ql-ui:before rules to avoid dual marker rendering and selector scope conflicts.
The new rules at lines 128–258 introduce two critical problems:
-
Dual marker rendering: Both
li.ordered.ql-indent-1::before(line 124) andli.ql-indent-1 .ql-ui:before(line 128) target the same list item. Since they render on different DOM elements (the<li>itself vs. its.ql-uichild span), both markers will display simultaneously if the.ql-uielement is present. -
Selector scope too broad: The new selector
li.ql-indent-1matches any<li>with that class, including unordered lists. From the DOM evidence, both<li class="ordered">and<li class="bullet">contain.ql-uichildren, meaning bullet lists will incorrectly display decimal counters via the new rules with!important, overriding any intended styling.
Additionally, all nine new rules use uniform decimal formatting, discarding the original visual hierarchy of alternating formats (lower-alpha, lower-roman, decimal). Consider either removing these new rules if the original li.ordered.ql-indent-n::before approach is now working, or replace them with a single SCSS mixin to reduce duplication and improve maintainability.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/fluent-editor/src/assets/editor.scss` around lines 128 - 130, The
new rules like "li.ql-indent-1 .ql-ui:before" are causing dual markers and
overbroad matching (affecting bullets) and overwrite the original ordered styles
("li.ordered.ql-indent-1::before"); remove or refactor those ".ql-ui:before"
rules so they don't conflict: either delete the nine "li.ql-indent-{1-9}
.ql-ui:before" selectors if the existing "li.ordered.ql-indent-{n}::before"
rules already render correctly, or replace them with a single SCSS mixin that
only applies to ordered lists (guarded by the ".ordered" class) and preserves
the alternating formats (lower-alpha, lower-roman, decimal) instead of forcing
decimal and using !important.
1 participant
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit