Skip to content

UI improvement: clearer icons and added text description#5425

Open
DedeHai wants to merge 3 commits intowled:mainfrom
DedeHai:newicons
Open

UI improvement: clearer icons and added text description#5425
DedeHai wants to merge 3 commits intowled:mainfrom
DedeHai:newicons

Conversation

@DedeHai
Copy link
Collaborator

@DedeHai DedeHai commented Mar 15, 2026

As discussed: adding two new icons and text under the buttons:

image

additional flash use: 380bytes.

I also added "how to" instructions for adding/changing icons in the future, the workflow is pretty straight forward but I had to figure it out first...

Summary by CodeRabbit

  • Documentation

    • Added a step-by-step guide for managing custom icon fonts and updated import/install instructions.
  • New Features

    • Added two icons (PixelForge, Editor) and expanded the icon set to 25 glyphs; replaced the icon selection with an updated collection.
    • Introduced new icon mappings and updated font assets used by the UI.
  • Style

    • Redesigned action button group into a centered three-item layout with labels.
    • Added new font-size utility classes and a small icon label style.
  • Bug Fix / Behavior

    • Removed automatic toggling of the palette editor from one effect-setting workflow.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 15, 2026

Walkthrough

Adds two new icons (pixelforge, editor), updates icon font files and base64 payloads, replaces the icon selection JSON, expands demo assets and font-size utilities, and restructures a centered three-item button group in the UI.

Changes

Cohort / File(s) Summary
Icon Font Documentation
wled00/data/icons-ui/HowTo_AddNewIcons.txt, wled00/data/icons-ui/Read Me.txt
Added HowTo for updating the custom icon font via IcoMoon and adjusted the documentation link and import instructions for selection.json.
Icon Font CSS & Mappings
wled00/data/icons-ui/style.css, wled00/data/index.css
Replaced @font-face sources and base64 payloads; added .i-pixelforge:before { content: "\e900"; } and .i-editor:before { content: "\e901"; }; added .iconlabel CSS class.
Icon Set Data
wled00/data/icons-ui/selection.json
Replaced the icon selection JSON with a new icon collection (new icons, IDs, names, metadata and codepoints).
Demos & Utilities
wled00/data/icons-ui/demo.html, wled00/data/icons-ui/demo-files/demo.css
Updated demo glyph count and grid sizes; added UI blocks for new icons; introduced .fs1 and .fs2 font-size utilities and adjusted .fs3 usage.
UI Layout & Behavior
wled00/data/index.htm, wled00/data/index.js
Replaced inline button group with a centered three-item layout (Files, PixelForge, Palettes) using .iconlabel; removed palette-editor visibility toggling from setEffectParameters.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • netmindz
🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'UI improvement: clearer icons and added text description' accurately reflects the main changes: adding new icons and text labels beneath tool buttons as described in the PR objectives.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@DedeHai DedeHai requested a review from softhack007 March 15, 2026 11:35
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Nitpick comments (2)
wled00/data/icons-ui/demo.html (1)

2-2: Consider adding lang attribute to <html> tag.

The opening <html> tag is missing the lang attribute. While this is an internal demo file (not production), adding it improves accessibility standards.

🔧 Optional fix
-<html>
+<html lang="en">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/data/icons-ui/demo.html` at line 2, Add a lang attribute to the
opening <html> tag in demo.html (the document root <html> element) — e.g., set
lang="en" or the appropriate language code — to improve accessibility and
assistive technology support; update the <html> tag declaration so the document
includes this attribute.
wled00/data/index.css (1)

3-3: Consider quoting the URL argument for consistency.

Stylelint flags the unquoted url() argument. While unquoted data URLs are valid CSS, quoting improves consistency and is a best practice per the function-url-quotes rule.

🔧 Optional fix
-  src: url(data:font/woff2;charset=utf-8;base64,d09GMgABAAAAAAsAAA0AAAAAFlgAAAqqAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP0ZGVE0cGhgGYACEGhEICp08lnkLPgABNgIkA3gEIAWDGweCNhvxEVGUcFI3wBeFsYOmlCFXadeSCl4PGhMTwyMh0q9d2MXuDaeszCMkmT3Abd0Eu2ijAIMUa1IDbaQmRj/wndtnJB+d8BHN/+ZKv+zJJpUCCAsMA5IcArBbtlteAg6ToYi3nPp6KxH97fd9OQgssMYTSymghAPMMODmLNpvv/P8BPzeodosVKppyCRNZE0QEqlTCp0SqP9T4O4gAMzzFuTJg2RPa6/23s/f4IYKREKfr6tTc/cLu7dh2JTwmhJdUiSLQqZVQFvmy6mScazQAwlZ7apjDAOl7l8dYEyN5azo7xRYCTCz7gCAzIa7hoI38uBn9/NfQMIrA5RCyCOfOtya0oEneAKP2+M8AEzujgX5QIQYkXEhC5nk4BVC6f6L4cmN4YazURxLPmVQjD4XkFWhNcfmv38+EMNisJkOyOKfgx6n/2z9efLjZY9fPol6EvJEdaY7I5y1zu3Ok64kl58r7bcprplPfZ+GvELuPwEAiGmvZJPj8ErdT9kXF+1jV7AvsG3seNY31uuFw5m/LLgKwNzGLGd8mO+cfw6A8S5jCsM/9wfEH8iWrJEYBLUxMHfsLJpcHQqzOuDEFhQjM1otoVvVg94O/zMIoCJtI1ACwThSfr8yQL1KvQ5rAApCJOJJKBSl4cdB3IwhcY7A5i3/hNDuIJq7NmfVLJNq2Z1hACMTkEpSDwPzGMtL6Qj7EFl8BemVw4zAppSRHW5ZhSxVZIZwKIDXduoxP57T0cgYeukZbC1afoHHq6/OwUSERJEC0lcLGXjp0QKyd7tOLYzdaXLTFHYixavtddgQ0YyI6xbZbLleW+DKSDGxqrvjTWIRtNNgOF6yGYc0ZhihH0R1vR9WuWn2Q4pkWdcmW0QsbEIYzglYJKxhzbvPBSWhn9uiMsuraZ3jiQ75dBgpD4mW9tgSdSHFzLzEcnLiNDvb59n9lVxzrObWObWDviOG3Dwt5RZCKdLLyl04L0+xvKG6aEG0nJFTM6AcuXROdpzmFJCcH9+uWfmohYxDH0Nxk+nRN4ZT3uJW3O32b9GChl57lSFlYeur0F6s+ve/cC8GeUHLy5CeTZoB7XGeFaxDWspDQ9CBaXdnUZU9hGerGTqIgUtgQxhFauojOOdYXo78csyahwycYlRk/FVxQdrYrQc7r1tJQJv1+Xi5FbW+xPCwj5pLicU1YATAPRM9hVc9RfdxWc2300x9lIgM3K/9xgtYHI8miESYICECeMSQt3EtAdq7jhUlLE2CiYgNqUeZNrzc9nLTTg+EeckP9Kz28vnwTeoolOtCGyF5WOonuVZNPkHX/RKff2/l48rnCUbIfJZad59cYhSwkWPEJUQkRvZrYUMVbAKCS6jB/bp7M2ItABfEMpgBinhBFLgze5jkAlW62xjORdV67XqlRsPsObLU7cI+/4ss97HdGJ2iXMrTFMuRTzAe2SISYd9NlE6rZmS4ahqS+8GKTA7ZuWs9YGQfYGQHdUqbXcy+iQC2aiEDhkdLTkhvpoYOmp6tTc6yvgVbEIGdkoPu2sV275V27N23h7awKFxyUm5n1CGxXfscu7nrlINyF7v00vEyotuwG5If4LpYtazK+s3xmi4bpC2UoPNVnRa9JubCZj3+jg4Zl+iGnds38V2bNqxnXOKcUkYv8Vw1vppL4+lMDDMok9jqbFmxHE1LeYp/Sc6O03odj1droeRpqckiE7Qa4jB+nO5OlVyIymtCtJdACJKcTKe3Kct4DL+2knGWW/gpzKXr191XULH0Ay1NmD9ndUMvJaoqrCq5dStqFaosxPyr8/N902gfWD5BcFtmaqreo8wxq1T1+g6+d8iQDLnRJBeYZP4jf/MEBpHR0Lj1zmvSecXw/+vqjLhyTs+enLSoujoiRy3LDbIhvmtxCTAzTZPZBNzr683+Pi7U/TOZjE+Z8yHfzlQzMbsdS4t1ulIwTTJN6/hj5nBM5GevHDFhfTVob+tnthVHUVyu6o1q8GeQCn5TYowqQ4a0asLK33fsSX2zLCVo473WZ4XPWu/gTUr4n1nSfH38mHmqzKpYCucxNo9yXJO1toU1NYX8GuAm7EXRRVH9ja9f0zCPBxUQoNvXeb64MoLftWmu23d39+9eBU1d+UObPKOkpETCw4F7hvbO3brNG0u0Qnrt6B9fveVI0AIMu+aSkOtc3VrSJG5IwMsAv2Rwvfs6ObS5xyXIGfFGlW5cxjv/b4+s7/gTclsCLce7ZvXo6i3rJxi2P9ln4irW+XW89OtSmD3FBmYRo9jaDUvEEip98Bf1mytr7BaFwmJXXVf/AsfRQx8c8MBdywDCjkgAM7s2GDeXXEdyeRSPy4viEmSqzesYgTclp1nKvv50S/kNN+Me01EF0wbWprFZyoBXWACDKu3Cljz17p1WbIZ7xFwjnWai0bGQqsZQK2xf3jggsrSXIVaxQ5EaS2GoE0/jlHG6deccNaU4PqGWZWrG4+588wUzl9saGzWaiLzKjH1B/XEQ4LgwcYIvPn16iYkW1K9gpBLXayyhAJWUWWu2o4jXaVtbtfzXgfuQTk3DaPbaBw817l7OvamJX0Yz0gPOtn4jx9N79MYQbCTF84i+sxz6kXTj3MYcbvkx56XzGsMoWng/EOvWrcWLo2/Jki/by8srSCjHsse7du1fBqtFNQfTTAMOYnfw+6srmZgvttlWFUunU+SoXWtJpU4qtduaqVndnxftCHhw2c5Qs43pa9cbRfu0y1Nt5oTN6hPvfS+w6LgjvexcaoGJZO76IeYh02unz5FWVjqiKer9q78ieyU0Da60eLSoAM296/BJHbMKCIXs4Xs17vLgTs35ikkIrh9BLc4dTXAxNvU5UvV1Vb7bhkO4BhD/9lGHO+/fn4NjlwtHhQO8BSSK5a9HRtGUqfZwnbmeTb2rTbpb764lHTY8Ydt87VtQbHW9UlkZn5WaPRqobxB3qLN+/cb18J+f+dNROn5AISbO1lVAbseul2ewdd4vjwdVkzC2L02fKWdJE3fnxAH7JhVtSF4/EDxhQNoukP0c++bTOk5j6JfTPn6EbndfYOD6FcsJIgKUob1Inz6u5zRZLPsWD0IB4t4DWzCg1XLY/wIGg30NHTTauPsJKVtSOtJ9O2/rYgfF03zzHqybNYqD/yx4tforP6Ld9vAr2ybl/3yIRTcdrwzuetFFSSMAH0LMxI2+fkDdCcDYJyA3ipitETmBOLi8EZmJSOpOPFq/DTxMGhrE3JLs83kymayp5Uh8Ms2xDiHtOJqBLNjEdz8eyLwgrYDkpX6syTp5sNVEYdFEZesHeyLOS68ey57lZy682pmLOqOJ4wcS2GwSmTfZWPLgMWbCdumm7N32YP5QDH110k4bAfiCL0Df065NIHyl/q626c2Y16wHeIviHYE4G+iT5oGtK/bUXlddcGyeJwQBPKxxgIKM7PhKE0/2uuQ+juqSmmzG3PDQFXfqjwMpWpmyPLpjTQbA8zda3OddU9za9W/xDBTYht7SfiikklBMEosFGw5ceGBX1J+TRABBhBBGBFHEEIeCD40EkkghjYx77NI+y02QY4JeWJYom4tVXCrlMg1XCDMwWSeBQMpFORkyRSehUM1EmQphXMqVogyVtJNIKOEiERcruUyTmZOVJOkkzsrJEGRl5WR5AgA=);
+  src: url("data:font/woff2;charset=utf-8;base64,d09GMgABAAAAAAsAAA0AAAAAFlgAAAqqAAEAAAAAAAAAAAAAAAAAAAAAAAAAAAAAP0ZGVE0cGhgGYACEGhEICp08lnkLPgABNgIkA3gEIAWDGweCNhvxEVGUcFI3wBeFsYOmlCFXadeSCl4PGhMTwyMh0q9d2MXuDaeszCMkmT3Abd0Eu2ijAIMUa1IDbaQmRj/wndtnJB+d8BHN/+ZKv+zJJpUCCAsMA5IcArBbtlteAg6ToYi3nPp6KxH97fd9OQgssMYTSymghAPMMODmLNpvv/P8BPzeodosVKppyCRNZE0QEqlTCp0SqP9T4O4gAMzzFuTJg2RPa6/23s/f4IYKREKfr6tTc/cLu7dh2JTwmhJdUiSLQqZVQFvmy6mScazQAwlZ7apjDAOl7l8dYEyN5azo7xRYCTCz7gCAzIa7hoI38uBn9/NfQMIrA5RCyCOfOtya0oEneAKP2+M8AEzujgX5QIQYkXEhC5nk4BVC6f6L4cmN4YazURxLPmVQjD4XkFWhNcfmv38+EMNisJkOyOKfgx6n/2z9efLjZY9fPol6EvJEdaY7I5y1zu3Ok64kl58r7bcprplPfZ+GvELuPwEAiGmvZJPj8ErdT9kXF+1jV7AvsG3seNY31uuFw5m/LLgKwNzGLGd8mO+cfw6A8S5jCsM/9wfEH8iWrJEYBLUxMHfsLJpcHQqzOuDEFhQjM1otoVvVg94O/zMIoCJtI1ACwThSfr8yQL1KvQ5rAApCJOJJKBSl4cdB3IwhcY7A5i3/hNDuIJq7NmfVLJNq2Z1hACMTkEpSDwPzGMtL6Qj7EFl8BemVw4zAppSRHW5ZhSxVZIZwKIDXduoxP57T0cgYeukZbC1afoHHq6/OwUSERJEC0lcLGXjp0QKyd7tOLYzdaXLTFHYixavtddgQ0YyI6xbZbLleW+DKSDGxqrvjTWIRtNNgOF6yGYc0ZhihH0R1vR9WuWn2Q4pkWdcmW0QsbEIYzglYJKxhzbvPBSWhn9uiMsuraZ3jiQ75dBgpD4mW9tgSdSHFzLzEcnLiNDvb59n9lVxzrObWObWDviOG3Dwt5RZCKdLLyl04L0+xvKG6aEG0nJFTM6AcuXROdpzmFJCcH9+uWfmohYxDH0Nxk+nRN4ZT3uJW3O32b9GChl57lSFlYeur0F6s+ve/cC8GeUHLy5CeTZoB7XGeFaxDWspDQ9CBaXdnUZU9hGerGTqIgUtgQxhFauojOOdYXo78csyahwycYlRk/FVxQdrYrQc7r1tJQJv1+Xi5FbW+xPCwj5pLicU1YATAPRM9hVc9RfdxWc2300x9lIgM3K/9xgtYHI8miESYICECeMSQt3EtAdq7jhUlLE2CiYgNqUeZNrzc9nLTTg+EeckP9Kz28vnwTeoolOtCGyF5WOonuVZNPkHX/RKff2/l48rnCUbIfJZad59cYhSwkWPEJUQkRvZrYUMVbAKCS6jB/bp7M2ItABfEMpgBinhBFLgze5jkAlW62xjORdV67XqlRsPsObLU7cI+/4ss97HdGJ2iXMrTFMuRTzAe2SISYd9NlE6rZmS4ahqS+8GKTA7ZuWs9YGQfYGQHdUqbXcy+iQC2aiEDhkdLTkhvpoYOmp6tTc6yvgVbEIGdkoPu2sV275V27N23h7awKFxyUm5n1CGxXfscu7nrlINyF7v00vEyotuwG5If4LpYtazK+s3xmi4bpC2UoPNVnRa9JubCZj3+jg4Zl+iGnds38V2bNqxnXOKcUkYv8Vw1vppL4+lMDDMok9jqbFmxHE1LeYp/Sc6O03odj1droeRpqckiE7Qa4jB+nO5OlVyIymtCtJdACJKcTKe3Kct4DL+2knGWW/gpzKXr191XULH0Ay1NmD9ndUMvJaoqrCq5dStqFaosxPyr8/N902gfWD5BcFtmaqreo8wxq1T1+g6+d8iQDLnRJBeYZP4jf/MEBpHR0Lj1zmvSecXw/+vqjLhyTs+enLSoujoiRy3LDbIhvmtxCTAzTZPZBNzr683+Pi7U/TOZjE+Z8yHfzlQzMbsdS4t1ulIwTTJN6/hj5nBM5GevHDFhfTVob+tnthVHUVyu6o1q8GeQCn5TYowqQ4a0asLK33fsSX2zLCVo473WZ4XPWu/gTUr4n1nSfH38mHmqzKpYCucxNo9yXJO1toU1NYX8GuAm7EXRRVH9ja9f0zCPBxUQoNvXeb64MoLftWmu23d39+9eBU1d+UObPKOkpETCw4F7hvbO3brNG0u0Qnrt6B9fveVI0AIMu+aSkOtc3VrSJG5IwMsAv2Rwvfs6ObS5xyXIGfFGlW5cxjv/b4+s7/gTclsCLce7ZvXo6i3rJxi2P9ln4irW+XW89OtSmD3FBmYRo9jaDUvEEip98Bf1mytr7BaFwmJXXVf/AsfRQx8c8MBdywDCjkgAM7s2GDeXXEdyeRSPy4viEmSqzesYgTclp1nKvv50S/kNN+Me01EF0wbWprFZyoBXWACDKu3Cljz17p1WbIZ7xFwjnWai0bGQqsZQK2xf3jggsrSXIVaxQ5EaS2GoE0/jlHG6deccNaU4PqGWZWrG4+588wUzl9saGzWaiLzKjH1B/XEQ4LgwcYIvPn16iYkW1K9gpBLXayyhAJWUWWu2o4jXaVtbtfzXgfuQTk3DaPbaBw817l7OvamJX0Yz0gPOtn4jx9N79MYQbCTF84i+sxz6kXTj3MYcbvkx56XzGsMoWng/EOvWrcWLo2/Jki/by8srSCjHsse7du1fBqtFNQfTTAMOYnfw+6srmZgvttlWFUunU+SoXWtJpU4qtduaqVndnxftCHhw2c5Qs43pa9cbRfu0y1Nt5oTN6hPvfS+w6LgjvexcaoGJZO76IeYh02unz5FWVjqiKer9q78ieyU0Da60eLSoAM296/BJHbMKCIXs4Xs17vLgTs35ikkIrh9BLc4dTXAxNvU5UvV1Vb7bhkO4BhD/9lGHO+/fn4NjlwtHhQO8BSSK5a9HRtGUqfZwnbmeTb2rTbpb764lHTY8Ydt87VtQbHW9UlkZn5WaPRqobxB3qLN+/cb18J+f+dNROn5AISbO1lVAbseul2ewdd4vjwdVkzC2L02fKWdJE3fnxAH7JhVtSF4/EDxhQNoukP0c++bTOk5j6JfTPn6EbndfYOD6FcsJIgKUob1Inz6u5zRZLPsWD0IB4t4DWzCg1XLY/wIGg30NHTTauPsJKVtSOtJ9O2/rYgfF03zzHqybNYqD/yx4tforP6Ld9vAr2ybl/3yIRTcdrwzuetFFSSMAH0LMxI2+fkDdCcDYJyA3ipitETmBOLi8EZmJSOpOPFq/DTxMGhrE3JLs83kymayp5Uh8Ms2xDiHtOJqBLNjEdz8eyLwgrYDkpX6syTp5sNVEYdFEZesHeyLOS68ey57lZy682pmLOqOJ4wcS2GwSmTfZWPLgMWbCdumm7N32YP5QDH110k4bAfiCL0Df065NIHyl/q626c2Y16wHeIviHYE4G+iT5oGtK/bUXlddcGyeJwQBPKxxgIKM7PhKE0/2uuQ+juqSmmzG3PDQFXfqjwMpWpmyPLpjTQbA8zda3OddU9za9W/xDBTYht7SfiikklBMEosFGw5ceGBX1J+TRABBhBBGBFHEEIeCD40EkkghjYx77NI+y02QY4JeWJYom4tVXCrlMg1XCDMwWSeBQMpFORkyRSehUM1EmQphXMqVogyVtJNIKOEiERcruUyTmZOVJOkkzsrJEGRl5WR5AgA=");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/data/index.css` at line 3, The unquoted data URL in the src: url(...)
declaration should be wrapped in quotes to satisfy stylelint's
function-url-quotes rule; update the src declaration (the src:
url(data:font/woff2... ) line) to use either url("data:font/woff2...") or
url('data:font/woff2...') so the data URI is quoted consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@wled00/data/icons-ui/HowTo_AddNewIcons.txt`:
- Line 7: Fix the typo in HowTo_AddNewIcons.txt by changing "Got to \"Generate
font\" tab, check unicodes are correct..." to "Go to \"Generate font\" tab,
check unicodes are correct..." so the instruction starts with "Go to" instead of
"Got to"; update the exact sentence in the file (the line containing the
"Generate font" instruction).

In `@wled00/data/index.htm`:
- Line 131: Remove the stray space in the HTML opening tag "<div >" so it's a
standard "<div>" tag; locate the occurrence of "<div >" in the markup (the
opening div element shown in the diff) and edit it to remove the extra space
before the closing angle bracket.

---

Nitpick comments:
In `@wled00/data/icons-ui/demo.html`:
- Line 2: Add a lang attribute to the opening <html> tag in demo.html (the
document root <html> element) — e.g., set lang="en" or the appropriate language
code — to improve accessibility and assistive technology support; update the
<html> tag declaration so the document includes this attribute.

In `@wled00/data/index.css`:
- Line 3: The unquoted data URL in the src: url(...) declaration should be
wrapped in quotes to satisfy stylelint's function-url-quotes rule; update the
src declaration (the src: url(data:font/woff2... ) line) to use either
url("data:font/woff2...") or url('data:font/woff2...') so the data URI is quoted
consistently.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7cb185de-c9f1-4378-92fa-9da09e77da80

📥 Commits

Reviewing files that changed from the base of the PR and between 7e490fc and a09acfc.

⛔ Files ignored due to path filters (4)
  • wled00/data/icons-ui/fonts/wled122.svg is excluded by !**/*.svg
  • wled00/data/icons-ui/fonts/wled122.ttf is excluded by !**/*.ttf
  • wled00/data/icons-ui/fonts/wled122.woff is excluded by !**/*.woff
  • wled00/data/icons-ui/fonts/wled122.woff2 is excluded by !**/*.woff2
📒 Files selected for processing (8)
  • wled00/data/icons-ui/HowTo_AddNewIcons.txt
  • wled00/data/icons-ui/Read Me.txt
  • wled00/data/icons-ui/demo-files/demo.css
  • wled00/data/icons-ui/demo.html
  • wled00/data/icons-ui/selection.json
  • wled00/data/icons-ui/style.css
  • wled00/data/index.css
  • wled00/data/index.htm

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
wled00/data/index.htm (1)

138-138: Minor: Inconsistent whitespace between attributes.

Line 138 has a tab character between type="button" and onclick=, while lines 132 and 144 use spaces between attributes. Consider using consistent spacing.

🔧 Suggested fix
-			<button class="btn btn-xs" title="PixelForge" type="button"	onclick="window.location.href=getURL('/pixelforge.htm')">
+			<button class="btn btn-xs" title="PixelForge" type="button" onclick="window.location.href=getURL('/pixelforge.htm')">
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@wled00/data/index.htm` at line 138, The button element with class "btn
btn-xs" and title "PixelForge" has a tab between the type="button" and onclick
attributes; replace the tab with a single space so attribute spacing matches the
other buttons (e.g., use a single space between type="button" and
onclick="window.location.href=getURL('/pixelforge.htm')") to ensure consistent
whitespace across attributes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@wled00/data/icons-ui/HowTo_AddNewIcons.txt`:
- Line 11: Update the sentence "in index.css, replace the font string at the
top, keep the "data:font/woff2;charset=utf-8;" and dont use octet-stream
(browser compatibility)." by capitalizing the first word ("In index.css") and
fixing the contraction ("don't") so it reads: In index.css, replace the font
string at the top, keep the "data:font/woff2;charset=utf-8;" and don't use
octet-stream (browser compatibility).

---

Nitpick comments:
In `@wled00/data/index.htm`:
- Line 138: The button element with class "btn btn-xs" and title "PixelForge"
has a tab between the type="button" and onclick attributes; replace the tab with
a single space so attribute spacing matches the other buttons (e.g., use a
single space between type="button" and
onclick="window.location.href=getURL('/pixelforge.htm')") to ensure consistent
whitespace across attributes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c1346327-3d57-4543-921e-910167c17e67

📥 Commits

Reviewing files that changed from the base of the PR and between a09acfc and cfaff6e.

📒 Files selected for processing (2)
  • wled00/data/icons-ui/HowTo_AddNewIcons.txt
  • wled00/data/index.htm

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@wled00/data/index.htm`:
- Around line 132-146: The three icon-only buttons that navigate to
getURL('/edit'), getURL('/pixelforge.htm'), and getURL('/cpal.htm') currently
rely only on title attributes; add an explicit accessible name by including
aria-label attributes (e.g., aria-label="File editor", aria-label="PixelForge",
aria-label="Custom palettes") on the corresponding <button> elements or add
visually hidden text inside each button to provide the same label so screen
readers get a reliable name.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: de72c5da-cf11-4990-9e8c-a46832b399cf

📥 Commits

Reviewing files that changed from the base of the PR and between cfaff6e and 8291211.

📒 Files selected for processing (2)
  • wled00/data/index.htm
  • wled00/data/index.js
💤 Files with no reviewable changes (1)
  • wled00/data/index.js

@softhack007
Copy link
Member

@DedeHai new icons look good 👍 Is there an impact on flash size?

@softhack007
Copy link
Member

@coderabbitai please verify that changes in this PR are compatible with common (modern!) browsers and WebView libraries. Also check conformance to relevant web standards like css, html, dom, javascript.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 15, 2026

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== index.htm (button group section) ==="
grep -n "iconlabel\|flex\|gap\|pixelforge\|editor\|Files\|PixelForge\|Palettes\|editPal\|btn" wled00/data/index.htm | head -60

echo ""
echo "=== index.css (iconlabel and font-face) ==="
grep -n "iconlabel\|font-face\|font-size\|margin-top\|woff2\|WIcons" wled00/data/index.css | head -40

echo ""
echo "=== style.css (icons-ui) ==="
grep -n "before\|pixelforge\|editor\|font-face" wled00/data/icons-ui/style.css | head -30

echo ""
echo "=== Full new button group HTML ==="
grep -n -A 40 "flex\|iconlabel" wled00/data/index.htm | head -80

echo ""
echo "=== index.js changes ==="
grep -n "editPal\|hide\|palette" wled00/data/index.js | head -20

Length of output: 12191


🏁 Script executed:

#!/bin/bash
echo "=== Check CSS iconlabel display/text-align ==="
grep -n -A5 "iconlabel" wled00/data/index.css

echo ""
echo "=== Check existing btn-xs styles (for sizing context) ==="
grep -n -A5 "btn-xs\b" wled00/data/index.css | head -30

echo ""
echo "=== Check style.css :before vs ::before ==="
grep -n "before" wled00/data/icons-ui/style.css

echo ""
echo "=== Check font-face declaration (MIME type) ==="
head -10 wled00/data/icons-ui/style.css

echo ""
echo "=== Check aria/accessibility attributes on icon <i> tags in new HTML ==="
grep -n "aria\|role\|alt" wled00/data/index.htm | head -20

echo ""
echo "=== Check wrapper div structure for iconlabel (full block) ==="
sed -n '128,155p' wled00/data/index.htm

Length of output: 3144


🏁 Script executed:

#!/bin/bash
echo "=== Check for global text-align in index.css (body/root/global rules) ==="
grep -n "text-align" wled00/data/index.css | head -20

echo ""
echo "=== Check for text-align on wrapping divs or flex children ==="
grep -n "text-align\|align-items\|align-self" wled00/data/index.css | head -20

echo ""
echo "=== Check Safari flex gap caniuse - verify gap has any fallback ==="
# Check if there's any existing fallback for gap in other flex containers
grep -n "gap\|column-gap\|row-gap" wled00/data/index.css | head -20

echo ""
echo "=== Check all aria-hidden usage in index.htm for pattern ==="
grep -n "aria-hidden\|role=" wled00/data/index.htm | head -20

echo ""
echo "=== Check entire CSS for `#btns` rules ==="
grep -n -A5 "#btns" wled00/data/index.css | head -30

Length of output: 1399


🏁 Script executed:

#!/bin/bash
echo "=== Check global text-align:center at line 51 ==="
sed -n '45,60p' wled00/data/index.css

echo ""
echo "=== Check line 163 text-align ==="
sed -n '158,170p' wled00/data/index.css

echo ""
echo "=== Confirm no gap fallback in index.css ==="
grep -c "gap" wled00/data/index.css

echo ""
echo "=== Check font-display usage in index.css font-face ==="
sed -n '1,15p' wled00/data/index.css

Length of output: 4756


@softhack007 Here is the compatibility and standards conformance analysis for this PR's changes:


Browser / WebView Compatibility & Standards Review

✅ Compatible & Conformant

Feature Details
display:flex + justify-content:center Chrome 29+, FF 28+, Safari 9+, Edge 12+ — universally safe
WOFF2 base64 data URL Correct MIME type data:font/woff2;charset=utf-8;...; Chrome 36+, FF 39+, Safari 12+, Edge 14+
type="button" on <button> Correct — prevents accidental form submission
title= attributes on buttons Valid HTML5 accessibility attribute
.iconlabel text centering body { text-align: center; } (Line 51 of index.css) propagates correctly — labels are centered by inheritance, no explicit text-align needed on .iconlabel
HTML5 structural validity Flex container with wrapper <div> children containing <button> + <div> is valid
JS change (index.js) Removal of two classList toggle lines — standard DOM API, no compatibility concerns

⚠️ Minor Notes

1. gap in Flexbox — Safari 14.0 (non-blocking)

The inline gap:16px on #btns is not supported in Safari 14.0 (flexbox gap was added in Safari 14.1, April 2021). Safari 14.0 supports gap only in Grid layouts. The pre-existing CSS rule #btns .btn-xs { margin: 0 4px; } provides minimal button spacing as an inadvertent fallback, so the UI remains functional. Safari 14.0 is old enough that this is unlikely to affect real users, but worth noting.

@softhack007 Here is the full compatibility and web standards analysis for this PR:


Browser / WebView Compatibility & Standards Review

✅ Fully Compatible & Standards Conformant

Feature Coverage
display:flex + justify-content:center Chrome 29+, FF 28+, Safari 9+, Edge 12+
@font-face + WOFF2 base64 data URL (data:font/woff2;charset=utf-8;...) Correct MIME type; Chrome 36+, FF 39+, Safari 12+, Edge 14+
type="button" on <button> Correct — prevents form submission
title= attributes on buttons Valid HTML5 accessibility
.iconlabel text centering body { text-align: center; } (Line 51 of index.css) already propagates globally — labels are correctly centered by inheritance, no additional text-align needed
HTML5 structural validity Flex container → wrapper <div><button> + <div> is valid
index.js change (removed 2 classList toggle lines) Standard DOM API — no compatibility concerns

⚠️ Minor: gap in Flexbox — Safari ≤ 14.0 / iOS ≤ 14.4

The inline gap:16px on the #btns container is the only real browser-compatibility note. Flexbox gap is not supported in Safari 3.2–14.0, with full support arriving in Safari 14.1+. On iOS, Safari gap in flex is supported from 14.5 onwards.

The pre-existing CSS rule #btns .btn-xs { margin: 0 4px; } (Line 853–854 of index.css) acts as an inadvertent fallback — on unsupported Safari versions the buttons will still appear with 4px side margins, just without the intended 16px gap. The UI remains fully functional; it's a minor cosmetic degradation only.

Note that the gap property before iOS 14.5 was implemented only for display: grid, and since both properties share the same gap name, a standard @supports (gap: 10px) check cannot reliably distinguish flex-gap support from grid-gap support — so a clean CSS-only fallback is non-trivial. Given that WLED targets modern browsers and Safari 14.0 is now ~5 years old, this is acceptable as-is.


ℹ️ Minor: aria-hidden on icon <i> elements (pre-existing pattern)

The new icon elements <i class="icons btn-icon">&#xe901;</i> use Private Use Area (PUA) Unicode codepoints. Without aria-hidden="true", screen readers may attempt to announce these characters. However, this is fully consistent with the existing codebase pattern (no aria-hidden usage exists anywhere in index.htm), and the buttons have title= attributes providing accessible names. This is not introduced by this PR.


ℹ️ Note: :before single-colon in icons-ui/style.css

The demo/documentation file wled00/data/icons-ui/style.css uses CSS2 single-colon :before selectors throughout. CSS3 specifies ::before (double-colon). However, this file is not part of the production build — the production font is embedded as a base64 data URL in index.css, and icons are referenced via HTML entities (&#xe900;, &#xe901;) directly. No impact on production behaviour.


Summary: The changes are compatible with all modern browsers and WebView libraries. The only noteworthy item is gap in flexbox on Safari ≤ 14.0 / iOS ≤ 14.4, which causes a minor cosmetic degradation (slightly tighter button spacing) but no functional breakage, and is acceptable for a modern-browser-targeted embedded web UI.


🧠 Learnings used
Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-13T17:45:14.848Z
Learning: Before marking work as complete, you MUST: (1) run `npm test` and ensure all tests pass, (2) build at least one hardware environment like `pio run -e esp32dev` successfully, and (3) for web UI changes, manually test the interface

Learnt from: CR
Repo: wled/WLED PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-03-13T17:45:14.847Z
Learning: Applies to wled00/data/**/*.js : Check for JavaScript errors in browser console after making changes to web UI JavaScript files

Learnt from: DedeHai
Repo: wled/WLED PR: 4798
File: wled00/FX.cpp:7531-7533
Timestamp: 2025-08-26T11:51:21.817Z
Learning: In WLED PR `#4798`, DedeHai confirmed that certain gamma-related calls in FX.cpp/FX_fcn.cpp/particle systems are intentional for effect-level shaping (e.g., brightness curves, TV sim, Pride 2015 pre-mix), distinct from final output gamma. Do not flag or remove these in future reviews; add comments when feasible to clarify intent.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:11.994Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with appropriate defined constants when those constants are meaningful in the context of the PR. For example, the hardcoded value 32 should be replaced with WLED_MAX_SEGNAME_LEN when it represents a segment name length limit. This improves code maintainability and reduces the risk of inconsistencies.

Learnt from: DedeHai
Repo: wled/WLED PR: 5372
File: wled00/FX.cpp:6489-6489
Timestamp: 2026-02-26T20:49:50.973Z
Learning: In WLED PR `#5372` (wled/WLED), mode_2Dscrollingtext in wled00/FX.cpp: the UI control backed by SEGMENT.custom2 is intentionally labeled “Font size”. It selects among predefined fonts whose pixel dimensions inherently define size; maintainers prefer to keep the label as “Font size” rather than “Font”.

Learnt from: softhack007
Repo: wled/WLED PR: 0
File: :0-0
Timestamp: 2025-11-14T13:37:30.955Z
Learning: In WLED code reviews, when code is modified or added, look for "magic numbers" (hardcoded numeric literals) and suggest replacing them with defined constants when meaningful constants exist in the codebase. For example, suggest replacing hardcoded "32" with WLED_MAX_SEGNAME_LEN if the context relates to segment name length limits.

@DedeHai
Copy link
Collaborator Author

DedeHai commented Mar 15, 2026

@DedeHai new icons look good 👍 Is there an impact on flash size?

about 380 bytes, see initial comment

Copy link
Member

@softhack007 softhack007 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants