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

Updating pxmagic and WLED UI #3296

Merged
merged 26 commits into from
Oct 21, 2023
Merged

Updating pxmagic and WLED UI #3296

merged 26 commits into from
Oct 21, 2023

Conversation

ajotanc
Copy link
Contributor

@ajotanc ajotanc commented Jul 20, 2023

Updating pxmagic and inserting it in the WLED UI with option to enable and disable.

@blazoncek
Copy link
Collaborator

blazoncek commented Jul 22, 2023

I've briefly tested this and have a few remarks (this is not a full review).

  1. First off, PXM or Pixel Magic button is inappropriate and IMO does not belong to the button bar.
    Screenshot 2023-07-22 at 12 21 41
  2. There is a lot of empty space at the top and the close button is missing (is rendered behind PXM content)
    Screenshot 2023-07-22 at 12 24 50
  3. When saving a preset, presets are reloaded before the actual preset is saved.
    Screenshot 2023-07-22 at 12 19 01
  4. PXM tool is reloaded every time it is activated which is a waste of resources.
  5. Istead of displaying empty "Select a choice" make uploading an image a default behaviour. Only display drop-down selector if there are actual images stored on device.
    Screenshot 2023-07-22 at 12 25 40
  6. Making "Individual" as a default Pattern is potentially problematic as the JSON size will increase beyond usability.
  7. Make sure you use tabs or spaces for indentation as appropriate for each file.

And honestly I do not see any reason to store images on the device as it only consumes FS space (image is stored in JSON format in preset) unless there was a usermod for displaying images in which case this tool would be obsolete.

EDIT: There is JS error when activating tool.
Screenshot 2023-07-22 at 12 19 48

@blazoncek
Copy link
Collaborator

Any news or updates @ajotanc ?

@blazoncek
Copy link
Collaborator

@ajotanc ?

@ajotanc
Copy link
Contributor Author

ajotanc commented Aug 22, 2023

@ajotanc ?

I had some personal problems, that's why I wasn't giving feedback, these days I'll see if I can update it and see what can be improved!

@softhack007 softhack007 added this to the 0.14.2 candidate milestone Sep 5, 2023
@blazoncek
Copy link
Collaborator

Hi @ajotanc,

When you'll be updating this PR/tool, please add documentation in a similar way as was done for PixelArt Converter.
https://kno.wled.ge/features/pixel-art-converter/

@blazoncek blazoncek marked this pull request as draft September 28, 2023 11:17
@ajotanc
Copy link
Contributor Author

ajotanc commented Sep 28, 2023

Hi @ajotanc,

When you'll be updating this PR/tool, please add documentation in a similar way as was done for PixelArt Converter. https://kno.wled.ge/features/pixel-art-converter/

How do I deal with this documentation issue? I'll go back and look at it today. I believe I already uploaded an update today

@blazoncek
Copy link
Collaborator

How do I deal with this documentation issue? I'll go back and look at it today. I believe I already uploaded an update today

Fork WLED-Docs, add description in relevant .md files nad then make a PR. Similar process as in main WLED repo.
@wled-faq or @dosipod or @scottrbailey may be able to help you better.

Topic 1, 2
I made the change, took it off the website and put it in the Preset section under the +Preset, +Playlist buttons, it was in a good location.

Topic 3, 4
It reloads because it needs the updated information so that it can create the preset correctly with all the information generated from the selected image.

Topic 5
The change has already been made, it just hasn't gone up yet, it's default upload

Topic 6, 7
Fixed
It was missing to upload this file in the commit.

- PXMagic is enabled by default in the Preset tab, and can be disabled in Settings, UI session
@blazoncek
Copy link
Collaborator

My suggestion would be to move the button in color/palette tab. Like so:
Screenshot 2023-10-15 at 21 53 08
Or on the other side next to +.

@ajotanc ajotanc marked this pull request as ready for review October 16, 2023 04:19
@dosipod
Copy link

dosipod commented Oct 16, 2023

Hi @ajotanc,
When you'll be updating this PR/tool, please add documentation in a similar way as was done for PixelArt Converter. https://kno.wled.ge/features/pixel-art-converter/

How do I deal with this documentation issue? I'll go back and look at it today. I believe I already uploaded an update today
@ajotanc
Hey , It is very simple to do docs as you already made a video which i prefer but KB is mostly done with pictures , i have made a draft from the doc of existing tool . located here https://github.com/dosipod/WLED-Docs/blob/master/docs/features/pxmagic.md . once your PR is finalize then we can finalize the doc for it along with pics ,video demo .etc . Does not harm to actually have the same instruction listed on both your site and KB . In all cases an instruction on your github page might be enough and more practical for faster updates as this is basically an external tool but that would mean you have to maintain an updated docs once you do a change as we have no write access to your repo to modify that

@ajotanc
Copy link
Contributor Author

ajotanc commented Oct 16, 2023

Hi @ajotanc,
When you'll be updating this PR/tool, please add documentation in a similar way as was done for PixelArt Converter. https://kno.wled.ge/features/pixel-art-converter/

How do I deal with this documentation issue? I'll go back and look at it today. I believe I already uploaded an update today
@ajotanc
Hey , It is very simple to do docs as you already made a video which i prefer but KB is mostly done with pictures , i have made a draft from the doc of existing tool . located here https://github.com/dosipod/WLED-Docs/blob/master/docs/features/pxmagic.md . once your PR is finalize then we can finalize the doc for it along with pics ,video demo .etc . Does not harm to actually have the same instruction listed on both your site and KB . In all cases an instruction on your github page might be enough and more practical for faster updates as this is basically an external tool but that would mean you have to maintain an updated docs once you do a change as we have no write access to your repo to modify that

I will do this by completing what was done, I ended up not touching it yet to make it functional first in the WLED UI and thus make it complete.

But I took a look at what was done and it was super interesting, I think I just need to complete it, it's very interesting how it was done.

I added the possibility of using the tooltip on buttons (.btn) with the span inside the button;
Adjusted the tooltip css to center 100% in the middle
Some cleaning and correction of sele
Since PXM will not open internally but on another page from the button, I decided to add the logo again if it's not a problem of course.. Just so I don't run out of information
@blazoncek blazoncek changed the base branch from main to 0_14_1 October 21, 2023 17:48
@blazoncek blazoncek merged commit 107bb14 into Aircoookie:0_14_1 Oct 21, 2023
9 of 12 checks passed
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