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

minor webui enhancements #4203

Open
wants to merge 4 commits into
base: 0_15
Choose a base branch
from

Conversation

maxi4329
Copy link

@maxi4329 maxi4329 commented Oct 17, 2024

Changes:

Added a black border around the white color quick-select in light mode.

Adjusted spacing between "Add segment" and "Reset segments" on 1440p screens (see images).

Fixed disappearing spaces after building by adding  .

Disabled letter input in the PIN field.

Expanded preset content to 100% width (see images).

Fixed font size in "Effect mode" label to match the other labels. And the margin between the labels ans the content

Before:
image

After:
image

Before:
image

After:
image

@blazoncek
Copy link
Collaborator

Thanks. CSS is a weird thing across different browsers.
Which browsers did you use for testing?

@maxi4329
Copy link
Author

Thanks. CSS is a weird thing across different browsers. Which browsers did you use for testing?

I used Chrome for testing.

@blazoncek
Copy link
Collaborator

Please use also Firefox, Safari and Edge. These are main browsers used.
You'll need mac and Windows versions.

@maxi4329
Copy link
Author

Please use also Firefox, Safari and Edge. These are main browsers used. You'll need mac and Windows versions.

I dont have a mac could I use an iPad (should also work since it uses webkit right?)

@blazoncek
Copy link
Collaborator

Test on as many as you can.

@maxi4329
Copy link
Author

maxi4329 commented Oct 17, 2024

Now i have tested it on the Windows, Android, and iPadOS versions of Chrome, Firefox, and Edge, as well as on Safari on iPadOS and the WLED Android app
Everything I changed worked on all of them couldn't find any visual bugs.

@@ -11,11 +11,20 @@
<style>
@import url("style.css");
</style>
<script>
function checkNum(o) {
const specialkeys = ["Backspace", "Tab", "Enter", "Shift", "Control", "Alt", "Pause", "CapsLock", "Escape", "Space", "PageUp", "PageDown", "End", "Home", "ArrowLeft", "ArrowUp", "ArrowRight", "ArrowDown", "Insert", "Delete"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

what does this line do and is it necessary or more of a best practice? it adds quite a lot of code...

Copy link
Collaborator

@blazoncek blazoncek left a comment

Choose a reason for hiding this comment

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

I do not like &nbsp; but at the same time I do not like it when browser joins two words with different style. There has to be a better solution than adding a bunch of &nbsp; into code as it increases flash use.

I also don't see it necessary to add onKeyDown event handling for pin code.

@maxi4329
Copy link
Author

I removed the onKeyDown event handling for the pin code.

This is the best solution I could find, but it's not perfect

  • it adds the non-breaking space before every <b> and <span> and after every <b> and <i> (even when it isn't needed)

  • they are still some spaces missing

@blazoncek
Copy link
Collaborator

Much better! 😄

@@ -44,6 +44,9 @@ button.sml {
min-width: 40px;
margin: 0 0 0 10px;
}
span:before, b:before, b:after, i:after{
content: "\00A0";
Copy link
Contributor

Choose a reason for hiding this comment

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

Where exactly were the spaces missing?
I found a problem with the minifyer that was removing spaces. I fixed it with #4267, so this may not be necessary anymore.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Screenshot 2024-11-10 at 18 08 34

Copy link
Contributor

@w00000dy w00000dy Nov 10, 2024

Choose a reason for hiding this comment

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

This is currently an issue with the minifyer. This will be fixed with PR #4267.

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.

5 participants