Skip to content

Changed HTML output to number input for Brightness, Speed, Intensity and Custom Sliders#4254

Closed
Svennte wants to merge 2 commits intowled:mainfrom
Svennte:Additional-slider-inputs
Closed

Changed HTML output to number input for Brightness, Speed, Intensity and Custom Sliders#4254
Svennte wants to merge 2 commits intowled:mainfrom
Svennte:Additional-slider-inputs

Conversation

@Svennte
Copy link
Contributor

@Svennte Svennte commented Nov 6, 2024

Enhancement: Convert HTML Output Elements to Number Inputs for Improved Precision

Sometimes, I struggled to input specific values using the range sliders for effects and brightness. To improve precision, I changed the HTML output elements for Brightness, Speed, Intensity, and Custom Sliders to number inputs. I kept changes minimal to fit within the current code structure.

Tested with

  • Branch: 0_15
  • Device: esp32dev

What Have I Changed

index.html

  • Deleted HTML output elements and inserted HTML number inputs.
  • Added this.value as an argument for the Effects/Brightness setter functions.
  • Added this.checkValidity() to ensure only valid values are accepted for number inputs.

index.css

  • Renamed .sliderbubble class to .sliderInput and adjusted styles accordingly.
  • Slightly reduced .sliderwrap class width to display the number input properly.
  • Added styles to visually indicate invalid values in slider number inputs

index.js

  • Added val as an argument for the Effects/Brightness setter functions, allowing both range and number inputs to function.
  • Changed updateTrail to update the input instead of the output.
  • Removed the now-unnecessary slider bubble event.
  • Removed the toggleBubble function.

Issues

  • The inputs function correctly across tested devices and browsers. However, in the WLED Android App, the on-screen keyboard doesn’t push the slider div to the top of the display (see screenshots below). This issue only affects the WLED Android App and does not occur in the WLED Native App or web clients.
  • Just tested on ESP32dev, no other environments
  • Not tested with apple devices/browser

Questions

I couldn't find functions that redefine min/max values for the range inputs. Is this correct, or is there something specific to handle min/max values that I may have overlooked?

Android
Firefox
Android_Firefox
WLED-Native
Android_WLED-Native
WLED-Standard
Android_WLED-Standard
Windows
Chrome
Windows_Chrome
Firefox
Windows_Firefox
Invalid value
Windows_InvalidValue

Summary by CodeRabbit

  • New Features

    • Added visible numeric input fields next to sliders, allowing direct entry and adjustment of values for brightness, speed, intensity, and custom parameters.
  • Style

    • Updated slider appearance to integrate numeric inputs and removed the old slider bubble tooltip.
    • Improved styling for numeric inputs, including clear invalid input indication.
  • Bug Fixes

    • Synchronized slider and numeric input values for a more consistent and user-friendly experience.

@Svennte Svennte changed the title Convert Brightness, Speed, Intensity, and Custom Sliders from HTML output to number input Changed HTML output to number input for Brightness, Speed, Intensity and Custom Sliders Nov 6, 2024
@softhack007
Copy link
Member

Thanks for your proposal!

personally I'm not an expert on html and JS, but I can answer one question

I couldn't find functions that redefine min/max values for the range inputs.

Ranges for all sliders (global brightness, effect sliders) are generally 0..255. The only exception is the "custom3" slider which has a reduced resolution of 0..31.

@Svennte
Copy link
Contributor Author

Svennte commented Nov 7, 2024

Ranges for all sliders (global brightness, effect sliders) are generally 0..255. The only exception is the "custom3" slider which has a reduced resolution of 0..31.

Okay, then the inputs are fine.
Is there something I have to do now?

@blazoncek
Copy link
Contributor

blazoncek commented Nov 11, 2024

Will fix #3019 but I would recommend restoring tabs which were replaced by spaces in JS file.

EDIT: I'd also try to reuse range from existing (hidden) input fields instead of introducing new ones.

@Svennte
Copy link
Contributor Author

Svennte commented Nov 11, 2024

Thanks for your recommendations!

> I would recommend restoring tabs which were replaced by spaces in JS file.
Tabs have been restored with the last commit.

> EDIT: I'd also try to reuse range from existing (hidden) input fields instead of introducing new ones.
While this is an interesting idea, I have some concerns about implementation. Here’s my reasoning:

  • Maintaining Readability and Manageability: Reusing input ranges would require detaching and reattaching inputs on each tab change, such as when switching between segment value inputs.
  • Complexity of Custom Logic: This approach would demand custom logic to dynamically set min/max values, and bind onchange, oninput, and possibly other functions.
  • Potential Future Issues: To ensure that future UI changes wouldn’t introduce unexpected issues, we’d likely need an input manager. This manager would handle adding, removing, and modifying inputs based on a config file for each tab.
  • Risk of Misuse Across Tabs: Without the input manager, it might be challenging to track which input is in use on each tab, especially as this approach scales to more inputs.

If you have a simpler solution in mind that I may have overlooked, please let me know. Given the required work and the limited benefits, this approach seems impractical, especially since the inputs don’t share the same parent element.

@netmindz netmindz changed the base branch from 0_15 to main December 16, 2024 13:27
@github-actions
Copy link

Hey! This pull request has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs.
Thank you for contributing to WLED! ❤️

@github-actions github-actions bot added the stale This issue will be closed soon because of prolonged inactivity label Apr 16, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Apr 16, 2025

Walkthrough

This change updates the slider UI in the WLED web interface by replacing the tooltip-style slider value bubble with a visible numeric input field adjacent to each slider. The CSS is revised to remove the .sliderbubble styles and introduce new styles for the .sliderInput numeric field. The HTML is modified to add <input type="number" class="sliderInput"> elements next to each slider, replacing previous <output> elements, and updates event handlers to synchronize values between sliders and numeric inputs. The JavaScript is refactored so setter functions now accept explicit value parameters and update the numeric input fields accordingly.

Changes

Files Change Summary
wled00/data/index.css Removes .sliderbubble styles, adds styles for .sliderInput numeric input, adjusts slider wrapper width, and updates media queries to target .sliderInput.
wled00/data/index.htm Replaces <output> slider bubbles with <input type="number" class="sliderInput">, updates event handlers to synchronize slider and numeric input values, and ensures validation.
wled00/data/index.js Removes touch event listeners for sliders, refactors setter functions (setBri, setSpeed, setIntensity, setCustom) to accept value parameters, updates logic to synchronize slider and numeric input values, and simplifies the toggleBubble and updateTrail functions.

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 18e0ec9 and f96710d.

📒 Files selected for processing (3)
  • wled00/data/index.css (4 hunks)
  • wled00/data/index.htm (2 hunks)
  • wled00/data/index.js (2 hunks)
🔇 Additional comments (16)
wled00/data/index.css (5)

697-711: Great addition of the new sliderInput style.

The new sliderInput class creates a visible, interactive number input field that replaces the previous sliderbubble tooltip. This is a good improvement for usability, allowing users to directly enter precise values instead of relying only on slider manipulation.


712-714: Good implementation of invalid input styling.

The red text for invalid inputs provides clear visual feedback to users when they enter values outside the allowed range.


445-445: Appropriate width adjustment for the slider wrapper.

Reducing the width from 230px to 222px helps accommodate the new number input field while maintaining the overall layout.


452-452: Updated comment reflects the new UI element.

The comment now correctly references the number input rather than the bubble that's been replaced.


1516-1518: Media query updates maintain consistency.

The media queries have been correctly updated to hide the new sliderInput elements on small screens, maintaining the same behavior previously applied to sliderbubbles.

Also applies to: 1577-1579

wled00/data/index.htm (6)

35-35: Well-implemented brightness slider with number input.

The range slider now updates the setBri function directly with its value, and the new number input includes validation, auto-selection on focus, and appropriate min/max constraints.

Also applies to: 38-38


205-205: Good implementation of speed control with number input.

The speed slider and number input are properly connected to the setSpeed function with appropriate validation.

Also applies to: 208-208


213-213: Well-implemented intensity slider with number input.

The intensity controls follow the same pattern as the others, maintaining consistency in the UI.

Also applies to: 216-216


221-221: Custom slider 1 properly implemented with number input.

The custom slider now includes a number input with validation before calling setCustom.

Also applies to: 224-224


229-229: Custom slider 2 properly implemented with number input.

The implementation correctly includes the necessary validation before updating values.

Also applies to: 232-232


237-237: Custom slider 3 correctly uses different min/max values.

This slider correctly uses a max value of 31 (instead of 255) as mentioned in the PR discussion from softhack007, and properly includes validation in the number input.

Also applies to: 240-240

wled00/data/index.js (5)

1134-1136: Good implementation of slider-to-input synchronization.

The updateTrail function now updates the input element when the slider changes, ensuring both UI elements stay in sync.


2363-2367: Well-refactored brightness setter function.

The setBri function has been updated to accept a value parameter, which makes it more reusable and decouples it from directly reading DOM elements.


2369-2373: Speed setter properly refactored to accept value parameter.

The setSpeed function now accepts a direct value parameter, improving its reusability.


2375-2379: Intensity setter properly refactored to accept value parameter.

The setIntensity function has been updated to accept a direct value parameter.


2381-2390: Custom value setter correctly refactored for explicit parameters.

The setCustom function has been updated to parse and use an explicit value parameter, and correctly assigns it to the appropriate custom property based on the index.

✨ Finishing Touches
  • 📝 Generate Docstrings

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot removed the stale This issue will be closed soon because of prolonged inactivity label Apr 17, 2025
@DedeHai
Copy link
Collaborator

DedeHai commented Jun 28, 2025

what is the status here?
#3170 seems related: would using that be a better approach?

@blazoncek
Copy link
Contributor

I would much prefer #3170 as it is simpler and universal.

@github-actions
Copy link

Hey! This pull request has been open for quite some time without any new comments now. It will be closed automatically in a week if no further activity occurs.
Thank you for contributing to WLED! ❤️

@github-actions github-actions bot added the stale This issue will be closed soon because of prolonged inactivity label Oct 26, 2025
@github-actions github-actions bot closed this Nov 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stale This issue will be closed soon because of prolonged inactivity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants