Skip to content
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

Added !NEW! SimpleColor action block #928

Open
wants to merge 11 commits into
base: stable
Choose a base branch
from
Open

Conversation

elsoazemelet
Copy link
Contributor

@elsoazemelet elsoazemelet commented Jan 29, 2025

The expected behaviour and design proposal is described in the ticket that this PR closes. See below.

Closes #912

Copy link
Contributor

github-actions bot commented Jan 29, 2025

Visit the preview URL for this PR (updated for commit 849ddc5):

https://grid-editor-web--pr928-feat-colorpicker-pymhtjs9.web.app

(expires Fri, 21 Feb 2025 14:31:03 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2b65ba6ef19c55d367eaffd04e46bcde25305d6f

@danim1130
Copy link
Contributor

Can we move the ColorPicker components to the uikit library?

@elsoazemelet elsoazemelet changed the title UI proposal Added !NEW! EasyColor action block Feb 12, 2025
@Greg-Orca Greg-Orca self-assigned this Feb 13, 2025
@Greg-Orca
Copy link
Contributor

Test Failed:

Issues:

  • Module LEDs do not receive the color.
  • Not working: Each active keyframe can be selected on the preview display, but R, G, B, and alpha values are not loaded for the given keyframe.
  • Cannot write RGB and alpha values in the text box; displays 'NaN'.

env: MacOS

Screen.Recording.2025-02-13.at.11.30.16.mov

@narayb
Copy link
Collaborator

narayb commented Feb 13, 2025

We'll issue an updated design for the colorbar as it looks too much like a slider and causes the user expectation that it's clickable and slide-able. That's on the design, we'll fix it.

Aside from this the feature looks as specified. Nice!

However I can only echo the sentiments of @Greg-Orca above:

  • slider does not update position or values when switching between color states min, mid or max
  • layer 0 does not exist, therefore it should not be a default layer (layers are 1 or 2)
  • typing into the value fields does not produce a change, fields become NaN
  • any kind of configuration created doesn't run on the nightly firmware currently, possibly awaiting a new FW patch to implement the functionality itself (this could be the cause of the above problems as well?)
  • the parameters should match already existing, similar blocks in design (LED Number input field followed by Layer input field)

kép

  • resizing the panel causes the colorwheel to grow unnecessarily large, the colorwheel size should probably stay static

color1

  • 'Easy Color' is not the name I would give to the block itself, something akin to Simple Color would suit it better
  • the randomization of a new added color wasn't part of the specification, and the way Editor hadles it causes it to change the configuration constantly

color2

Also the sequence of events when adding new colors is wrong. Now what it does is:
1, Has a color at the max, then you add a second color.
2, The color previously at the max moves to the min position.

What it should be doing instead:
1, Has a color at the max, then you add a second color.
2, The color previously at the max should still stay at the max position and the second color should move into the min position.

@elsoazemelet elsoazemelet changed the title Added !NEW! EasyColor action block Added !NEW! SimpleColor action block Feb 17, 2025
@elsoazemelet
Copy link
Contributor Author

elsoazemelet commented Feb 17, 2025

  • Color picker Square Y axis and Circle radius should set brightness 50 to 100 and render picker accordingly
  • Selection order when adding new should follow visual adding (max, min, middle)
  • Selection should be more visible (not pretty)
  • Fix square pickers out of bound selection
  • Make VM more lightweight

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ready for QA
Development

Successfully merging this pull request may close these issues.

Implement new color action block based on reference design
4 participants