Update toolbox icon/text padding#659
Conversation
📝 WalkthroughWalkthroughUpdated Blockly CSS theming infrastructure across multiple color modes (dark, light, contrast, low-vision) with token standardization, expanded mobile toolbox layout and search overlay styling, and introduced mobile/light-aware category background tinting in JavaScript using conditional CSS ChangesBlockly Theming & Mobile Toolbox Improvements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
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)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
style/blockly.css (1)
325-337:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore a positioned ancestor for the toolbox search row.
The input stays
position: absolute, but these overrides remove the wrapper’s positioning context withposition: static. Without a newposition: relativeon the search category, the field can anchor to the toolbox/viewport instead of its own row, which makes the search box jump to the wrong place on desktop and mobile.Suggested fix
+.blocklyToolbox .blocklyToolboxCategory:has(input[type='search']) { + position: relative; +} + .blocklyTreeRowContentContainer input[type='search'] { position: absolute; top: 0; left: 0;Also applies to: 723-728, 788-800
🤖 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 `@style/blockly.css` around lines 325 - 337, The search input is absolutely positioned but its wrapper lost positioning context, causing it to anchor to the toolbox/viewport; restore a positioned ancestor by adding position: relative to the search row container (e.g. the .blocklyTreeRowContentContainer or the specific search category wrapper used alongside the selector .blocklyTreeRowContentContainer input[type='search']) so the input is positioned relative to its row; apply the same fix to the other similar rule blocks noted (lines around 723-728 and 788-800) to ensure consistent behavior on desktop and mobile.
🤖 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 `@style/blockly.css`:
- Around line 511-558: The stylelint errors come from illegal quoting of the
Asap font family and a compact block comment; update all font-family
declarations (e.g., in selectors body .blocklyText, body .blocklyTreeLabel,
body[data-theme='contrast'] .blocklyText, body[data-theme='dark']
.blocklyTreeLabel, etc.) to remove quotes around Asap and ensure proper
comma/space formatting (e.g., 'Atkinson Hyperlegible Next', Asap, Helvetica,
Arial, Lucida, sans-serif) and expand or reformat any compact block comment to
satisfy stylelint spacing rules (use standard /* ... */ formatting with
spaces/newlines) so the hunk no longer triggers stylelint.
---
Outside diff comments:
In `@style/blockly.css`:
- Around line 325-337: The search input is absolutely positioned but its wrapper
lost positioning context, causing it to anchor to the toolbox/viewport; restore
a positioned ancestor by adding position: relative to the search row container
(e.g. the .blocklyTreeRowContentContainer or the specific search category
wrapper used alongside the selector .blocklyTreeRowContentContainer
input[type='search']) so the input is positioned relative to its row; apply the
same fix to the other similar rule blocks noted (lines around 723-728 and
788-800) to ensure consistent behavior on desktop and mobile.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3e6378ac-4ee3-40a2-a195-38877ae1c677
📒 Files selected for processing (2)
style/blockly.csstoolbox.js
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body .blocklyText { | ||
| font-weight: 500 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body .blocklyTreeLabel { | ||
| font-weight: 500 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body[data-theme="contrast"] .blocklyText { | ||
| body[data-theme='contrast'] .blocklyText { | ||
| font-weight: 600 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body[data-theme="dark-contrast"] .blocklyText { | ||
| body[data-theme='dark-contrast'] .blocklyText { | ||
| font-weight: 600 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body[data-theme="dark-contrast"] .blocklyTreeLabel { | ||
| body[data-theme='dark-contrast'] .blocklyTreeLabel { | ||
| font-weight: 500 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body[data-theme="contrast"] .blocklyTreeLabel { | ||
| body[data-theme='contrast'] .blocklyTreeLabel { | ||
| font-weight: 500 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body[data-theme="dark"] .blocklyText { | ||
| body[data-theme='dark'] .blocklyText { | ||
| font-weight: 500 !important; | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; | ||
| } | ||
|
|
||
| body[data-theme="dark"] .blocklyTreeLabel { | ||
| body[data-theme='dark'] .blocklyTreeLabel { | ||
| font-family: | ||
| "Atkinson Hyperlegible Next", "Asap", Helvetica, Arial, Lucida, sans-serif !important; | ||
| 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important; |
There was a problem hiding this comment.
Clear the stylelint errors in this hunk.
The quoted Asap family name and the compact block comment still trip the current stylelint rules, so this file will keep reporting static-analysis errors until those are cleaned up.
Suggested fix
- 'Atkinson Hyperlegible Next', 'Asap', Helvetica, Arial, Lucida, sans-serif !important;
+ 'Atkinson Hyperlegible Next', Asap, Helvetica, Arial, Lucida, sans-serif !important;- /*pointer-events: auto;*/
+ /* pointer-events: auto; */Also applies to: 799-799
🧰 Tools
🪛 Stylelint (17.12.0)
[error] 511-511: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 517-517: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 523-523: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 529-529: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 535-535: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 541-541: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 547-547: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 553-553: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
[error] 558-558: Expected no quotes around "Asap" (font-family-name-quotes)
(font-family-name-quotes)
🤖 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 `@style/blockly.css` around lines 511 - 558, The stylelint errors come from
illegal quoting of the Asap font family and a compact block comment; update all
font-family declarations (e.g., in selectors body .blocklyText, body
.blocklyTreeLabel, body[data-theme='contrast'] .blocklyText,
body[data-theme='dark'] .blocklyTreeLabel, etc.) to remove quotes around Asap
and ensure proper comma/space formatting (e.g., 'Atkinson Hyperlegible Next',
Asap, Helvetica, Arial, Lucida, sans-serif) and expand or reformat any compact
block comment to satisfy stylelint spacing rules (use standard /* ... */
formatting with spaces/newlines) so the hunk no longer triggers stylelint.
1 participant
Summary
Attempt to fix the look and feel of the toolbox icons on mobile. I think this has been broken by Blockly 13.
AI usage
Claude Sonnet 4.6 used throughout, everything run past a human and approved individually. Design by a human.
Summary by CodeRabbit