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

WebHost: Remove "Random" as an option and move to separate button in Player Settings #944

Merged
merged 4 commits into from
Nov 13, 2022

Conversation

ThePhar
Copy link
Member

@ThePhar ThePhar commented Aug 21, 2022

Putting this in draft as it will definitely need styling to be consistent with other themes, but here's a working prototype of the separate random button in the player settings page.

The goal is to reduce confusion with what the "Random" option is on Choices in the player settings page and instead moves it to its own button off to the side. By clicking the button, it locks out user input on the normal input and lights up to indicate that option is now being randomized.

An additional benefit is players can now randomize Ranges in addition to Choices/Toggles:
image

Preview in motion:
randomize

Final YAML export based on above gif:

Starcraft 2 Wings of Liberty:
  progression_balancing: random
  accessibility: random
  game_difficulty: random
  upgrade_bonus: random
  bunker_upgrade: random
  all_in_map: random
  mission_order: random
  shuffle_protoss: random
  relegate_no_build: random
description: 'Generated by https://archipelago.gg/'
game: Starcraft 2 Wings of Liberty
name: Phar

The random option is also saved in local storage so coming back to the page retains the saved setting.

Somethings I still want to work on is some code clean up and working with Farrak on styles to maintain better styling. :)

Verified

This commit was signed with the committer’s verified signature. The key has expired.
BridgeAR Ruben Bridgewater
…Player Settings
@ThePhar ThePhar added the is: enhancement Issues requesting new features or pull requests implementing new features. label Aug 21, 2022
@ThePhar ThePhar requested a review from LegendaryLinux August 21, 2022 01:08
@ThePhar
Copy link
Member Author

ThePhar commented Aug 21, 2022

Did not mean to pull in the review request (since it's in Draft) whoops. Not sure how that happened.

@LegendaryLinux
Copy link
Member

So far it looks great. Let me know when you're ready for review, then after it's accepted I'll take it to the designer and get the theme styles.

@black-sliver
Copy link
Member

I think user expectation of a "dice button" would be to roll immediately, but with it being "pressed in" and the corresponding input disabled, i think it's clear what it does when pressing it, so 👍 from me

@LegendaryLinux
Copy link
Member

I agree with that. At first I thought it was a button to immediately randomize the current value of the option. I know a checkbox was briefly discussed, and the button looks better. I still hope we come up with a clear way to explain what the button does up front, without the user needing to "figure it out."

@ThePhar
Copy link
Member Author

ThePhar commented Aug 22, 2022

The mouseover tooltip should be sufficient in my opinion. It can be tweaked if the verbiage should be clearer.

@ThePhar
Copy link
Member Author

ThePhar commented Aug 22, 2022

The other alternative I thought of is to spell out "Randomize Setting" or something, but that makes the button or checkbox larger of which there isn't much space of anyway, so outside of users reading the guides, this seemed like the best option. If it helps, it's the same strategy used by the Varia rando team on their site, only without the tooltip on mouseover.

image

@LegendaryLinux
Copy link
Member

I think if the background-color of the button is made to match the color of the tooltip question marks, we should be in good shape.

@LegendaryLinux
Copy link
Member

Maybe also change the tooltip on the button to read "Toggle randomization for this option!"

Copy link
Member

@LegendaryLinux LegendaryLinux left a comment

Choose a reason for hiding this comment

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

Sorry for the very long turnaround on this. I added a couple comments about some pretty simple changes that should help it look good across themes.

@LegendaryLinux LegendaryLinux added the affects: webhost Issues/PRs that touch webhost and may need additional validation. label Oct 6, 2022
@ThePhar ThePhar self-assigned this Nov 1, 2022

Verified

This commit was signed with the committer’s verified signature. The key has expired.
BridgeAR Ruben Bridgewater
@ThePhar ThePhar marked this pull request as ready for review November 4, 2022 21:29

Verified

This commit was signed with the committer’s verified signature. The key has expired.
BridgeAR Ruben Bridgewater

Verified

This commit was signed with the committer’s verified signature. The key has expired.
BridgeAR Ruben Bridgewater
@ThePhar
Copy link
Member Author

ThePhar commented Nov 4, 2022

Made those changes, let's see how this looks.

@LegendaryLinux LegendaryLinux merged commit 4d79920 into ArchipelagoMW:main Nov 13, 2022
@ThePhar ThePhar deleted the randomize-button branch November 13, 2022 05:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects: webhost Issues/PRs that touch webhost and may need additional validation. is: enhancement Issues requesting new features or pull requests implementing new features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants