-
-
Notifications
You must be signed in to change notification settings - Fork 9
rename UI components #75
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Front-end ======== - MultiInput to FieldRenderer - Array / MultiRow to RowRenderer - FileEdit to FileEditWidget - EditObject to EditRowWidget Back-End ======== - App_DATE: YYYYMMDD - ModuleIO: gpio: ledPins - Nodes: use header
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Documentation Updates docs/components.md, docs/develop/modules.md, docs/moonbase/filemanager.md, docs/moonbase/inputoutput.md, src/MoonBase/Modules/FileManager.h |
Updated docs and comments to reflect renames: MultiInput→FieldRenderer, Array→RowRenderer, FileEdit→FileEditWidget; minor text refinements and anchor/link updates. |
Svelte component files interface/src/lib/components/moonbase/FieldRenderer.svelte, interface/src/lib/components/moonbase/FileEditWidget.svelte, interface/src/lib/components/moonbase/RowRenderer.svelte, interface/src/lib/components/moonbase/EditRowWidget.svelte |
Replaced imports/usages of old components with their renamed counterparts (FieldRenderer, RowRenderer, FileEditWidget, EditRowWidget); updated header comments and a runtime log string. |
Route / Page updates interface/src/routes/moonbase/filemanager/FileManager.svelte, interface/src/routes/moonbase/module/Module.svelte |
Updated imports and template usages to the new component names (FieldRenderer, RowRenderer, FileEditWidget) while preserving props/bindings. |
Backend pin management refactor src/MoonBase/Modules/ModuleIO.h |
Renamed PinAssigner::assignPin parameter (pinNr→gpio_num) and converted index-based loops over pin arrays to range-based iteration across many board configurations. |
Node internals refactor src/MoonBase/Nodes.cpp |
Replaced repeated deep member access with a local LightsHeader* header pointer and adjusted reads/writes to use this local reference for color correction, presets, and offsets. |
Driver comment tweak src/MoonLight/Nodes/Drivers/D_FastLED.h |
Minor comment update in a switch-case to reference ledPins[pinIndex] instead of pinNr. |
Build metadata platformio.ini |
Updated APP_DATE build flag from "2025121511" to "20251216". |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
- Areas requiring extra attention:
src/MoonBase/Modules/ModuleIO.h: confirm all callers and call sites compile afterassignPinparameter rename and that range-based loops preserve semantics across board configurations.interface/...Svelte files: ensure no stale imports/usages remain and that event/prop bindings are intact after component substitutions.src/MoonBase/Nodes.cpp: validate the localLightsHeader*usage preserves intended behavior and side effects.
Possibly related PRs
- Module: support button, new rows, nested tables #1 — touches MultiInput/Array UI components; likely related to the component rename/substitution work.
- Board preset add presets #70 — overlaps changes to ModuleIO pin assignment signature and pin loop traversal; directly related.
- Board presets p4 #74 — modifies MultiInput-related UI components; related to the renaming/refactor of UI components.
Suggested labels
enhancement
Poem
🐰 Renamed the fields with a hop and a twitch,
FieldRenderer, RowRenderer, a clean little switch.
FileEditWidget popped into view,
Pins now iterate fresh and true,
I nibble docs and code—what a delicious fix!
Pre-merge checks and finishing touches
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
|
| Title check | ❓ Inconclusive | The title 'rename UI components' is vague and generic. It uses non-descriptive terminology that doesn't convey specific information about which components are renamed or what the new names are. | Consider a more specific title like 'Rename UI components: MultiInput→FieldRenderer, MultiRow→RowRenderer, FileEdit→FileEditWidget, EditObject→EditRowWidget' to clearly communicate the scope of changes. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✨ Finishing touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
ui-rendering
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 help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/moonbase/inputoutput.md (1)
78-78: LGTM - Minor grammatical improvement.The textual refinement from "set also" to "also set" improves readability.
Optional: Consider fixing markdown indentation.
The static analysis tool flagged that this line has incorrect indentation (4 spaces instead of the expected 2 for nested list items). While this doesn't affect functionality, fixing it would improve markdown consistency.
docs/components.md (1)
170-174: Adjust list indentation for consistency.The nested list items use 4-space indentation instead of the expected 2-space indentation, which is flagged by markdownlint.
Apply this diff to fix the indentation:
* Show multiple rows of objects - * Summary and zoom + * Summary and zoom * Recursive - * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node + * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node * Uses FieldRenderersrc/MoonBase/Modules/ModuleIO.h (1)
478-479: Refactor this loop for consistency.This loop uses
sizeof(ethernetPins)instead of a range-based for loop like the other refactored loops in this PR. While it happens to work (6 bytes for 6 uint8_t elements), it's:
- Type-dependent and would break if the array type changed
- Inconsistent with the modernized loops throughout this file
Apply this diff to align with the other loop refactorings:
-uint8_t ethernetPins[6] = {28, 29, 30, 31, 34, 35}; -for (int i = 0; i < sizeof(ethernetPins); i++) pinAssigner.assignPin(ethernetPins[i], pin_Ethernet); +uint8_t ethernetPins[] = {28, 29, 30, 31, 34, 35}; +for (uint8_t gpio : ethernetPins) pinAssigner.assignPin(gpio, pin_Ethernet);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
docs/components.md(1 hunks)docs/develop/modules.md(1 hunks)docs/moonbase/filemanager.md(1 hunks)docs/moonbase/inputoutput.md(1 hunks)interface/src/lib/components/moonbase/EditRowWidget.svelte(2 hunks)interface/src/lib/components/moonbase/FieldRenderer.svelte(4 hunks)interface/src/lib/components/moonbase/FileEditWidget.svelte(4 hunks)interface/src/lib/components/moonbase/RowRenderer.svelte(5 hunks)interface/src/routes/moonbase/filemanager/FileManager.svelte(3 hunks)interface/src/routes/moonbase/module/Module.svelte(2 hunks)platformio.ini(1 hunks)src/MoonBase/Modules/FileManager.h(1 hunks)src/MoonBase/Modules/ModuleIO.h(10 hunks)src/MoonBase/Nodes.cpp(4 hunks)src/MoonLight/Nodes/Drivers/D_FastLED.h(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/interface/**/*.{svelte,ts,tsx,js,json} : Modify frontend code in the `interface/` directory using SvelteKit
Applied to files:
interface/src/routes/moonbase/module/Module.svelteinterface/src/routes/moonbase/filemanager/FileManager.sveltedocs/develop/modules.mdinterface/src/lib/components/moonbase/FileEditWidget.svelte
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/interface/src/routes/**/*.svelte : Create Svelte routes in `interface/src/routes/` for different pages of the web application
Applied to files:
interface/src/routes/moonbase/module/Module.svelteinterface/src/routes/moonbase/filemanager/FileManager.sveltedocs/develop/modules.md
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/interface/src/lib/components/**/*.svelte : Reusable Svelte components should be placed in `interface/src/lib/components/`
Applied to files:
interface/src/routes/moonbase/module/Module.svelteinterface/src/routes/moonbase/filemanager/FileManager.sveltedocs/develop/modules.mdinterface/src/lib/components/moonbase/FileEditWidget.svelte
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/interface/src/lib/stores/**/*.{ts,tsx} : Store global application state using Svelte stores in `interface/src/lib/stores/`
Applied to files:
interface/src/routes/moonbase/module/Module.svelteinterface/src/routes/moonbase/filemanager/FileManager.svelte
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/interface/svelte.config.js : Configure SvelteKit build and adapter settings in `interface/svelte.config.js`
Applied to files:
docs/develop/modules.md
📚 Learning: 2025-11-29T16:17:30.619Z
Learnt from: CR
Repo: MoonModules/MoonLight PR: 0
File: misc/parking/GEMINI.md:0-0
Timestamp: 2025-11-29T16:17:30.619Z
Learning: Applies to misc/parking/**/platformio.ini : Use PlatformIO to build and upload the ESP32 backend C++ code
Applied to files:
platformio.ini
📚 Learning: 2025-12-07T14:16:03.228Z
Learnt from: ewowi
Repo: MoonModules/MoonLight PR: 0
File: :0-0
Timestamp: 2025-12-07T14:16:03.228Z
Learning: VirtualLayer mappingTableSize is uint16_t with a maximum of 65535 LEDs. If a user defines a space where size.x * size.y * size.z exceeds 65535, the code accepts the overflow and performs mapping partially. This is an explicit design decision balancing performance and memory constraints.
Applied to files:
src/MoonBase/Nodes.cpp
🪛 LanguageTool
docs/moonbase/inputoutput.md
[grammar] ~78-~78: Ensure spelling is correct
Context: ...100W LED power; change if needed. * Switch1: (also set the switches on the board) ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
docs/moonbase/inputoutput.md
78-78: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
docs/components.md
171-171: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
173-173: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (29)
src/MoonBase/Modules/FileManager.h (1)
22-22: ✓ Documentation update correctly reflects component rename.The comment on line 22 has been updated to reference
FileEditWidgetinstead ofFileEdit, aligning with the PR's component renaming objectives. The documentation link anchor correctly points to the renamed component. No behavioral changes.platformio.ini (1)
59-59: LGTM - Build date update.The APP_DATE build flag has been updated to match the PR date (2025-12-16). This is a routine version tracking change.
docs/moonbase/filemanager.md (1)
7-7: LGTM - Documentation updated to reflect component rename.The documentation correctly references the renamed
FileEditWidgetcomponent, maintaining consistency with the codebase changes.src/MoonLight/Nodes/Drivers/D_FastLED.h (1)
533-533: LGTM - Improved comment clarity.The comment now accurately references
ledPins[pinIndex]instead of the genericpinNr, making it more precise and helpful for code maintainers.docs/develop/modules.md (1)
147-147: LGTM - Documentation updated for component rename.The documentation correctly references
FieldRenderer.svelte(previouslyMultiInput.svelte), maintaining alignment with the codebase refactoring.interface/src/lib/components/moonbase/FieldRenderer.svelte (4)
3-3: LGTM - File header updated.The file header comment correctly reflects the new component name
FieldRenderer.svelte.
15-15: LGTM - Import updated for component rename.The import statement correctly references the renamed
FileEditWidgetcomponent.
38-38: LGTM - Log message updated.The log message now correctly identifies the component as "FieldRenderer" for clearer debugging.
129-129: LGTM - Component usage updated.The component instantiation correctly uses
<FileEditWidget>consistent with the renaming.interface/src/lib/components/moonbase/RowRenderer.svelte (4)
25-28: LGTM - Imports updated for component renames.Both import statements correctly reference the renamed components:
EditObject→EditRowWidgetMultiInput→FieldRenderer
85-85: LGTM - Modal invocation updated.The
modals.open()call correctly referencesEditRowWidget(previouslyEditObject).
181-188: LGTM - Filter component updated.The search filter section correctly uses
<FieldRenderer>(previously<MultiInput>), maintaining all existing props and bindings.
217-224: LGTM - Item rendering updated.The item rendering loop correctly uses
<FieldRenderer>(previously<MultiInput>) for displaying non-array/control/password fields.interface/src/lib/components/moonbase/EditRowWidget.svelte (4)
1-10: LGTM - File header added.The addition of a standard file header comment block improves code documentation and maintains consistency with other component files.
18-19: LGTM - Imports updated for component renames.Both import statements correctly reference the renamed components:
MultiInput→FieldRendererMultiRow→RowRenderer
64-70: LGTM - Array rendering updated.The
<RowRenderer>component (previously<MultiRow>) is used correctly with all existing props and bindings preserved.
74-85: LGTM - Field rendering updated.All usages of
<FieldRenderer>(previously<MultiInput>) are correctly updated throughout the template, maintaining identical props and bindings.src/MoonBase/Nodes.cpp (2)
366-398: LGTM! Refactoring improves code readability.Introducing a local pointer to
layerP.lights.headerreduces verbosity and improves maintainability without changing behavior.
401-558: LGTM! Consistent refactoring pattern.The local
headerpointer is used consistently throughout the method for cleaner code.interface/src/routes/moonbase/module/Module.svelte (2)
10-12: LGTM! Component imports updated correctly.The imports reflect the component renames from MultiInput to FieldRenderer and MultiRow to RowRenderer.
200-208: LGTM! Component usage updated correctly.Component tags updated to match the renamed imports while preserving all props and bindings.
interface/src/routes/moonbase/filemanager/FileManager.svelte (3)
33-35: LGTM! Component imports updated correctly.Imports reflect the systematic component renames across the codebase.
274-279: LGTM! FileEditWidget usage updated correctly.Component tag and props preserved during the rename.
367-371: LGTM! FieldRenderer usage updated correctly.Component tag and props preserved during the rename.
interface/src/lib/components/moonbase/FileEditWidget.svelte (3)
3-6: LGTM! Metadata updated to reflect new component name.File header comments correctly reference the new FileEditWidget name.
23-23: LGTM! Import updated correctly.Import statement reflects the MultiInput to FieldRenderer rename.
182-208: LGTM! All FieldRenderer usages updated correctly.All three instances (Name, Contents, Upload) preserve their props and event handlers during the component rename.
src/MoonBase/Modules/ModuleIO.h (2)
214-221: LGTM! Parameter rename improves clarity.Renaming
pinNrtogpio_numfollows GPIO naming conventions and makes the code more readable.
285-285: LGTM! Loop modernization improves code quality.Converting index-based loops to range-based for loops is idiomatic modern C++ and reduces potential off-by-one errors.
Also applies to: 321-321, 335-335, 418-418, 430-430, 441-441, 462-462, 489-489, 493-494
|
@coderabbitai, could you review the new naming conventions:
Does it make sense, is it consistent and is it a solid / commonly accepted design pattern? |
|
Tip For best results, initiate chat on the files or code changes. @ewowi, I'll review these naming convention changes for you. Overall Assessment: ✅ Solid ImprovementThe new naming conventions are well-thought-out, consistent, and align with commonly accepted design patterns in modern UI frameworks. Here's my detailed analysis: Pattern Analysis1. Renderer Pattern (
|
| Old Name | New Name | Clarity Gain |
|---|---|---|
MultiInput |
FieldRenderer |
✅ "Multi" is vague; "Renderer" indicates purpose |
Array |
RowRenderer |
✅ "Array" is too generic; "RowRenderer" is domain-specific |
MultiRow |
RowRenderer |
✅ Removes redundant "Multi" prefix |
FileEdit |
FileEditWidget |
✅ Makes UI component nature explicit |
EditObject |
EditRowWidget |
✅ "Row" is more specific than "Object" |
Design Pattern Compliance
✅ Separation of Concerns: The distinction between "Renderer" (presentation) and "Widget" (interaction) suggests good architectural separation.
✅ Self-Documenting Code: New names require less context to understand their purpose.
✅ Framework Agnostic: These patterns are recognized across multiple UI frameworks (React, Angular, Vue, Flutter, etc.).
Minor Suggestion
Consider documenting the distinction between "Renderer" and "Widget" components in your architecture docs:
- Renderer: Focused on displaying/presenting data with minimal interaction
- Widget: Complex, interactive components with editing capabilities
This will help future contributors understand when to use each suffix.
Verdict
The renaming is consistent, semantically clear, and follows industry-standard patterns. It's a solid refactoring that will improve code maintainability and developer experience. 🎉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/components.md(1 hunks)src/MoonBase/Modules/ModuleIO.h(11 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/components.md
165-165: Images should have alternate text (alt text)
(MD045, no-alt-text)
174-174: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
docs/components.md (2)
162-163: Excellent addition of Renderer vs. Widget distinction.The new top-level descriptors clearly communicate the semantic difference between these two component categories, which aligns with the recommendation from the previous review. This will help guide future contributors in choosing appropriate naming conventions.
167-186: Component renaming is semantically clear and well-documented.The updates to FieldRenderer, RowRenderer, and the new Widget components (EditRowWidget, FileEditWidget) are well-explained. Descriptions accurately reflect the new naming and clarify their purpose and relationships.
src/MoonBase/Modules/ModuleIO.h (4)
198-198: LGTM: Documentation updated to reflect UI component rename.The comment correctly references
RowRenderer, consistent with the PR's objective to renameArray/MultiRow→RowRenderer.
214-220: LGTM: Parameter rename improves clarity.Renaming
pinNrtogpio_numis more descriptive and aligns with the naming convention used elsewhere in the codebase (e.g., line 238). The internal accesses are correctly updated to match.Note: This changes the public method signature of
PinAssigner::assignPin. If this class is used outside this file, external callers will need updates.
285-285: LGTM: Range-based loops improve code clarity and safety.Converting index-based loops to range-based for loops is an idiomatic C++11+ improvement that eliminates index arithmetic and potential off-by-one errors. The type is consistent (
uint8_t gpiomatches the array element type), and the logic is preserved.Note: Line 355 is in commented code, but the refactoring maintains consistency.
Also applies to: 321-321, 335-335, 355-355
418-418: LGTM: Consistent range-based loop refactoring across all board configurations.The refactoring consistently applies range-based for loops across all remaining board-specific pin configurations (MHCV43, MHCP4NanoV1, TroyP4Nano, AtomS3, Cube202010). The pattern handles multiple pin array types (
ledPins,exposedPins,ethernetPins) uniformly, maintaining type safety and preserving the original logic.Also applies to: 424-424, 430-430, 441-441, 462-462, 478-479, 489-489, 494-494
| Renderer: Takes JSON definitions and based on the type property shows a specific widget (e.g. input, checkbox, dropdown, ...) | ||
| Widget: Complex, interactive components with editing capabilities | ||
|
|
||
| <img width="30" src="https://github.com/user-attachments/assets/b0e8af99-ed76-422a-8bd1-bfbd9e0f4c44"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add alt text to the image for accessibility.
The image on line 165 is missing alt text, which is required for accessibility compliance.
-<img width="30" src="https://github.com/user-attachments/assets/b0e8af99-ed76-422a-8bd1-bfbd9e0f4c44"/>
+<img width="30" alt="MoonBase components icon" src="https://github.com/user-attachments/assets/b0e8af99-ed76-422a-8bd1-bfbd9e0f4c44"/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img width="30" src="https://github.com/user-attachments/assets/b0e8af99-ed76-422a-8bd1-bfbd9e0f4c44"/> | |
| <img width="30" alt="MoonBase components icon" src="https://github.com/user-attachments/assets/b0e8af99-ed76-422a-8bd1-bfbd9e0f4c44"/> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
165-165: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
In docs/components.md around line 165, the <img> tag is missing alt text which
breaks accessibility; update the tag to include a concise, descriptive alt
attribute (e.g., alt="Short description of image") that accurately describes the
image content for screen readers and assistive technologies.
| * Summary and zoom | ||
| * Recursive | ||
| * An array can have an array e.g. multiple nodes with multiple controls per node | ||
| * Uses MultiInput | ||
| * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix list indentation to match markdown standards.
The unordered list items under RowRenderer are indented with 4 spaces when they should use 2 spaces per markdown formatting rules.
### RowRenderer
-* Show multiple rows of objects
- * Summary and zoom
+* Show multiple rows of objects
+ * Summary and zoom
* Recursive
- * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node
+ * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Summary and zoom | |
| * Recursive | |
| * An array can have an array e.g. multiple nodes with multiple controls per node | |
| * Uses MultiInput | |
| * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node | |
| ### RowRenderer | |
| * Show multiple rows of objects | |
| * Summary and zoom | |
| * Recursive | |
| * A row renderer can have a row renderer e.g. multiple nodes with multiple controls per node |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
174-174: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
176-176: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🤖 Prompt for AI Agents
In docs/components.md around lines 174 to 176, the unordered list under
"RowRenderer" uses 4-space indentation which doesn't follow Markdown standards;
change those nested list lines to use 2 spaces (or a single tab equivalent) for
their indentation so the list renders correctly (e.g., indent child list items
by two spaces relative to their parent) and ensure consistent spacing for all
list items in that block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coderabbitai, regarding
The unordered list items under RowRenderer are indented with 4 spaces when they should use 2 spaces per markdown formatting rules.
If I indent with 2 spaces it doesn't look good, I need 4 to show it the right way indented.
Can you check why that is and confirm about the standard: 2, 3 or 4 spaces?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
Markdown nested list indentation 2 or 4 spaces CommonMark GitHub
💡 Result:
Short answer: Neither CommonMark nor GitHub require a fixed 2‑ or 4‑space rule. Instead they use the column of the first non‑whitespace character after the list marker as the “baseline” for that list item, and continuation / nested blocks must be indented to at least that column. That means 2 spaces will work in many common cases, but other cases (ordered lists with wider markers, or extra spaces after the marker) require 3, 4, 5, etc. spaces. [1][2][3]
Examples and references:
- CommonMark spec (list-item indentation = position of first non‑space after marker). [1]
- GitHub Flavored Markdown follows the same approach (GFM notes and examples). [2]
- Discussion / history showing why a rigid “2 vs 4” rule was rejected and how 2‑space often works in practice. [3]
Sources:
[1] CommonMark spec — Lists / list items. (spec.commonmark.org)
[2] GitHub Flavored Markdown (GFM) spec — lists examples. (github.github.io)
[3] CommonMark discussion: why fixed 2/4‑space rules aren’t used. (talk.commonmark.org)
If you want, I can show minimal examples that illustrate when 2 spaces works and when more are needed.
Citations:
- 1: https://spec.commonmark.org/0.31.2/?utm_source=openai
- 2: https://github.github.io/gfm/?utm_source=openai
- 3: https://talk.commonmark.org/t/could-also-support-two-spaces-for-list-indentation/2022?utm_source=openai
@ewowi, you're right to question this! The CommonMark and GitHub Flavored Markdown specifications don't actually mandate a fixed 2 or 4 space rule. Instead, they use the column of the first non-whitespace character after the list marker as the baseline, and nested blocks must be indented to at least that column.
In practice, this means:
- 2 spaces works for simple unordered lists with single-character markers (
*or-) - 4 spaces may be needed for proper alignment in certain cases, especially with ordered lists or when you want visual clarity
The markdownlint tool's MD007 rule is being prescriptive with its default configuration (expecting 2 spaces), but if 4 spaces renders correctly in your documentation viewer and looks better, then it's perfectly valid.
My recommendation: Since the content renders properly with 4 spaces and that's what looks good in your documentation, you have two options:
-
Configure markdownlint to allow 4-space indentation by adding to your
.markdownlint.json:{ "MD007": { "indent": 4 } } -
Ignore this particular rule for list indentation if you prefer flexible spacing based on what renders well.
The standard is flexible by design, so trust what renders correctly in your documentation viewer over the linter's default preference. 👍
Summary by CodeRabbit
Documentation
New Features
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.