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
feat: Settings Screen Redesign #2335
base: main
Are you sure you want to change the base?
Conversation
See above for updated version.Settings screen
SettingsRedesign.mp4 |
@JamieCallan117 This looks like major improvements in usability! |
No longer in this pr, was added in #2502Overlay Selection screen
OverlaySelectionRedesign.mp4 |
These changes all look great! I am very happy to see that you're taking the time needed to make these improvements. |
Up to you, but I have one idea for the overlay management screen. Do you think we could add user-specified/optionally custom naming for the custom overlays? It needs a bit of background work before we could do it though. |
Also, all of the new designs look beautiful! Thanks for sorting out all the UIs I "quickly" added so we could work on other core parts of the mods. |
I don't know if it is the right way, but would simply adding a new config field for all custom overlays called |
I have seen this requested a lot so yes it definitely could be done, and as Rafii said, I think a simple config field should get the job done |
The redesign is now going to be done in seperate parts so once a certain area is done it can be merged. This pr will focus on the "Wynntils" area, those being Settings, Overlays and Crowd Sourcing |
No longer in this pr, was added in #2502Overlay Management Screen
OverlayManagementRedesign.mp4 |
Just letting you know I started reviewing this, but it is a bit too long to check in a single run. I'll be back. |
common/src/main/java/com/wynntils/core/consumers/overlays/CustomNamedOverlay.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/wynntils/core/persisted/config/Config.java
Outdated
Show resolved
Hide resolved
common/src/main/java/com/wynntils/screens/settings/WynntilsBookSettingsScreen.java
Outdated
Show resolved
Hide resolved
Ah didn't realise there was a config with text long enough to overflow, I'll make it scrolling text as all of the buttons and input widgets are the same size for consistency.
It uses the same scrolling as the settings screen which is the same used on tm and poi manager so not sure why that screen specifically wouldn't work well, I can double check the implementation but I don't have a laptop currently set up for me to test it out on |
common/src/main/java/com/wynntils/core/persisted/config/ConfigManager.java
Show resolved
Hide resolved
Scrolling feels good now! Also checked the button sizes, they look good. |
Essentially, this looks good to me, if we can get the textures added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've glanced through all code, and studied some parts in more particular. I think this looks good.
6c6f9fc
to
1c6c3d8
Compare
…se scrolling text for GeneralSettingsButton
effd9bc
to
964b25b
Compare
I've removed the overlay and crowd sourcing redesigns as one was added separately and the other I'll do another time as not a fan of how it was done here. There's also a few other fixes but no major changes from the original but might be worth giving it another review anyway. I also failed at squashing so can mostly ignore the first ~15 commits |
So do we have textures now, or what happened? |
Still no textures, Rafii has started some of them but no promises |
At some points it's easier to make a diff relative to main, and apply that diff to a new branch and create a new PR. I think that is easier than trying to play with git's rewrite functionality. (Rebasing/squashing an open PR is also taboo; that makes the reviews hopeless to follow later on.) |
feat: Textures and adjustments for Settings screen
Final Textures
SettingsScreenRedesign.mp4