Skip to content

Conversation

@w00000dy
Copy link
Contributor

@w00000dy w00000dy commented Nov 9, 2023

This turns the original UI into a simplified UI instead of having a second simplified UI. Now in the simple ui there is so much duplicate code and I think this would be a better implementation of the simplified ui. So my idea was that when the simple ui is enabled, we fetch the default ui, hide all unnecessary ui elements and then show the ui.

This is what it looks like:

grafik
grafik

grafik
grafik

grafik grafik
grafik grafik
grafik

I ended up not having a button to open the palettes or effects because it just adds an unnecessary click that you have to do to change your effect or palette. In the old simplified UI, you then had to scroll as well, so it makes no difference if you always have palettes and effects visible or not.

Now, because the simplified ui takes up much less space: Should we make the simplified ui compiled in by default?

@blazoncek
Copy link
Contributor

I was under the impression that you were about to remove simple.htm and accompanying files.

@w00000dy
Copy link
Contributor Author

I wanted to get rid of the duplicate code.

@blazoncek
Copy link
Contributor

The solution looks interesting but I see no point in having two files to maintain (if you override one's layout). Could you not put everything into index.js and only have decision made on some status in Info object? Admittedly it does not exist ATM, but that can be fixed. It will also make wled_server.cpp a bit cleaner.

Question: Shall we introduce a new class (i.e. "nosimple") that can be used to hide (or otherwise manipulate) elements not belonging into simplified user interface?

I have yet to test it but from your screenshots I cannot see simplified segments. Are they intentionally hidden? IMO if there is just one force it's opacity to 255 and state to on and you can hide it, but if there are 2 or more, display their power buttons and opacity (brightness) sliders.

@w00000dy
Copy link
Contributor Author

At first I also thought about putting everything in a file, but since there was no information in the Info object about whether the simplified UI was enabled or not, I changed my mind and did it this way. But since you would also prefer to have everything in one file I will add to the Info object whether the simplified UI is turned on or not and based on this info in index.js display the simplified UI or not.

I'm not sure whether the new class is absolutely necessary, because only display is actually set to none. But I'll just see during development whether it's needed or not.

The simplified segments are not implemented yet because I didn't quite understand what you meant, but now I do. I will add that.

@blazoncek
Copy link
Contributor

One more question: Could you condense palettes and effects into a searchable drop-down list?

@w00000dy
Copy link
Contributor Author

Yes, I can do that. 👍🏼

@w00000dy
Copy link
Contributor Author

I moved everything to index.js and implemented the simplified segments and added a drop-down button for the palettes.
Basically I did that what @blazoncek suggested here:

... Could you not put everything into index.js and only have decision made on some status in Info object? Admittedly it does not exist ATM, but that can be fixed. ...

Here are some screenshots of what it looks like now:
grafik
grafik
grafik
grafik
grafik
grafik
grafik

grafik grafik
grafik grafik
grafik grafik

I have two questions:

  1. Do we really need an effects drop-down? I don't see the benefit of having one. Changing the effect is such a basic thing that you probably do every time you open the UI. And if we add a drop-down list for that, you would have to do an extra click every time you want to change the effect. Do you see this the same way?
  2. In the current simple UI, the button that opens the drop-down menu has the name of the current palette/effect. I think this can be a bit confusing. For example, if you have selected the Wavesins effect, a less tech-savvy user might be confused as to what this button does.
    grafik
    So I changed the text of the button to Change palette / Hide palettes.
    What do you think? Is that better?

I am open to suggestions for improvement and feedback. 😄

@blazoncek
Copy link
Contributor

  1. I agree but I still think the list may be too long (180+ effects in total)
  2. not if styling would be different IMO

Nevertheless it is nice to see this progress.
I can't wait to find the time to put it through my fingers. 😉

@blazoncek
Copy link
Contributor

@WoodyLetsCode ❤️

I finally had the time to testdrive this PR. Marvelous job!

Still, I'd do this (no segment names and thinner segment spacing):
Screenshot 2023-11-22 at 15 55 23

And I found this visual error (incorrect width of palettes):
Screenshot 2023-11-22 at 15 46 50

I do miss presets (at least quick loads are there) but I get there needs to be some sort of compromise.

@Aircoookie and @softhack007 please review and comment.

@blazoncek
Copy link
Contributor

Perhaps:

#segcont.simple .lstI {
	margin-top: 4px;
	min-height: unset;
}

@w00000dy
Copy link
Contributor Author

Thanks @blazoncek for testing!

I will change the segments according to your screenshot. 👍🏼

I will also look into the visual error, I couldn't reproduce it at the moment.

I thought it would be better to just have the quick load of presets, because that would lead to a minimalized and simpler UI. But if you want to have presets in the simple UI, I could add that as well.

I'm currently torn between two design ideas. Which one do you prefer?
The text above shows the current palette:
grafik
As it is in the current simple ui:
grafik

Or do you have other ideas? I'm open to that as well.

@blazoncek
Copy link
Contributor

I prefer my solution, but I leave it to you.
Reasoning: Users (especially inexperienced) will get used to tap/click twice to get to palettes or effects. But presets are there for a reason (most likely created by someone else for those users) and should be accessed with a single tap/click.

@Moustachauve
Copy link
Contributor

I'd just like to give my opinion on the design of the palette selector.
Based on the screenshots only, it looks weird. It just makes separated boxes of palette appear in the nornal page?

I would have two alternatives suggestions

  1. Make it look like a dropdown menu, with a different background and no space between the elements. I would also drop the round borders
  2. Show the palette in a "dialog" on top of the whole user interface with a title "Select a palette" and a button "cancel" at the bottom.

@blazoncek
Copy link
Contributor

@Moustachauve that would be preferable but would also mean (I think) to make two versions of UI (as was the case with simple.htm).
What @WoodyLetsCode is trying to achieve is to modify existing UI with CSS and a bit of JS to be displayed in a simplified way.

@Moustachauve
Copy link
Contributor

Moustachauve commented Nov 22, 2023

It could be done by switching some CSS classes in JS I believe.

@blazoncek
Copy link
Contributor

It could be done by switching some CSS classes in JS I believe.

Please help. 😄

@Moustachauve
Copy link
Contributor

I don't know how to contribue to this PR (I couldn't push), but I made this change here to turn the palette selection into a dialog:
Moustachauve@a389e41

It's an early prototype that could probably be improved. Here's what it looks like:
image

@blazoncek
Copy link
Contributor

I don't know how to contribue to this PR (I couldn't push),

Add a new remote (Woody's WLED.git) to your fork (git remote add WoodyLetsCode https://github.com/WoodyLetsCode/WLED.git), checkout his branch (git checkout -b simple-mode WoodyLetsCode/simple-mode), make changes, push to your fork (git push origin), make a PR to Woody's fork. It is much easier to do that mumbo-jumbo using Git client in VSC.

@w00000dy
Copy link
Contributor Author

Thank you @Moustachauve ! ❤️

Your screenshot looks really impressive! I can't wait to get a PR from you.

@blazoncek
Copy link
Contributor

@WoodyLetsCode if you're not afraid you can grant @Moustachauve collaborator rights to your repo. It will be faster developing in tandem that way. You can revoke at any time.

@blazoncek
Copy link
Contributor

BTW I'd take the dropdown approach for both: palettes and effects. I'd leave effect sliders on screen at all times.

@w00000dy
Copy link
Contributor Author

w00000dy commented Nov 23, 2023

Good idea. I have granted @Moustachauve collaborator rights.

@Moustachauve
Copy link
Contributor

Moustachauve commented Nov 23, 2023

I made PR w00000dy#2 in your repo

@w00000dy
Copy link
Contributor Author

Thank you @Moustachauve !
I took a look at it and there seems to be something wrong.

@w00000dy
Copy link
Contributor Author

@blazoncek Do you want to have this in 0_14_1 or 0_15 branch. I'm not sure because @softhack007 added this to the 0.14.2 candidate milestone.

@blazoncek
Copy link
Contributor

0_15
It is too great change to be included in 0.14.x

@blazoncek blazoncek removed this from the 0.14.2 candidate milestone Nov 30, 2023
@w00000dy
Copy link
Contributor Author

w00000dy commented Dec 6, 2023

I also added the dropdown menu for effects.
I'm still not sure which I like better. 🤔 The implementation with or without dropdown for the effects. Without dropdown, you need one click less to set the effect. But it looks cleaner with dropdown. 🤷🏼
But I'm going to keep it that way, because @blazoncek prefers it that way.

@blazoncek
Copy link
Contributor

But I'm going to keep it that way, because @blazoncek prefers it that way.

Looks good.
There is one issue: scroller is inaccessible (cannot use mouse to drag scroller in FF).

I'd also like to see active palette and effect displayed on the buttons instead of "Change ..."
Screenshot 2023-12-06 at 18 59 02

@blazoncek
Copy link
Contributor

One more thing. Buttons for pixel art, and custom palettes (add/delete) can be hidden too.

@w00000dy
Copy link
Contributor Author

w00000dy commented Dec 8, 2023

I think we are done. 🎉
Here are two screenshots of what it looks like now:
grafik
grafik

@w00000dy w00000dy marked this pull request as ready for review December 8, 2023 12:16
@blazoncek
Copy link
Contributor

Tested and approved.
@softhack007 and @Aircoookie please give it a try.

@WoodyLetsCode the scroll bar in a popup window for effects or palettes still cannot be accessed using mouse in Firefox. Scrolling with mouse wheel does work though. I have not yet tested it on mobile so it may need further tweaking.

@w00000dy
Copy link
Contributor Author

w00000dy commented Dec 9, 2023

@blazoncek Can you tell me which version of Firefox you are using and which operating system you use? On my Firefox it works after my commit 930a1ae

@blazoncek
Copy link
Contributor

Using FF 115.5.0esr on macOS 10.14

@w00000dy
Copy link
Contributor Author

It seems that there is a difference between Firefox for Windows and Firefox for MacOS.
I need to find a solution for this.

@w00000dy
Copy link
Contributor Author

w00000dy commented Dec 12, 2023

@blazoncek It should work on macOS now.

I also made a few other small changes:
The background of the dialog is now blurred instead of darkened.
This is how it looks now:
grafik
grafik

@Moustachauve
Copy link
Contributor

@WoodyLetsCode Blur looks good on a photo! I wonder what does it look like on the default single color background? Does it have enough contrast?

@w00000dy
Copy link
Contributor Author

w00000dy commented Dec 13, 2023

@Moustachauve This is how it looks like on the default single color background:
grafik

Here in light mode:
grafik

I think it also looks good there.

@blazoncek blazoncek merged commit 37c9fd2 into wled:0_15 Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants