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

Use Equipment confirmation modal for equipping arti/weapons #2054

Open
frzyc opened this issue May 1, 2024 · 2 comments
Open

Use Equipment confirmation modal for equipping arti/weapons #2054

frzyc opened this issue May 1, 2024 · 2 comments
Labels
needs more thought Needs more thought to flesh out the solution

Comments

@frzyc
Copy link
Owner

frzyc commented May 1, 2024

When swapping artifact/weapons, you are presented with this screen:

image Clicking on the green will pop up a comparison, and then upon equip, will show this screen: image whereas clicking on the Red will swap without popping up the build confirmation screen.

Since this build confirmation screen is very useful, we should standardize it, and apply it to both equipment path.

Question: does this make sense to also apply this to the equipment dropdown in artifactcard/weaponcard as well?

@frzyc frzyc added the good first issue Issue with low complexity and relatively easy solution for those looking to raise an easy PR label May 1, 2024
@frzyc
Copy link
Owner Author

frzyc commented May 1, 2024

Here is another thought, can be combine the "Confirm Equipment" Modal with the "build diff" modal? they share a lot of the same UI elements, it is not a stretch to combine them. Would require breaking up BuildEquipModal into its pieces so its easier to be put together for different context.

@frzyc frzyc added needs more thought Needs more thought to flesh out the solution and removed good first issue Issue with low complexity and relatively easy solution for those looking to raise an easy PR labels May 1, 2024
@arquelion
Copy link
Contributor

Probably a good idea to standardize the build swap confirmation for consistency's sake as well. I dunno how often users accidentally equip an artifact/weapon or equip to the wrong build whether by directly clicking on it or misclicking in the dropdown, but in theory, this would guard against that too. That being said, BuildDisplayItem didn't have the option to equip at all prior to teams/loadouts, and the comparison UI piggybacks off it, so maybe it's worth asking whether having the option to equip both through clicking on the card itself and clicking "Compare to current build" then "Equip to current build" is a little redundant now.

I can see some potential value in merging BuildEquipModal in with the confirmation since presumably the reason a user might want to see an individual comparison is that they're already considering an artifact/weapon swap (but doesn't necessarily care about being optimal, else they would be in the Optimize tab and not here), and if they're satisfied then they can immediately swap without clicking on another button first. However, like I mentioned above, the comparison UI currently piggybacks off BuildDisplayItem, so if going this route, I'd probably pull the comparison itself out into its own thing and create a new modal that combines the comparison with elements from BuildEquipModal instead of actually directly inserting the pieces into BuildDisplayItem and then having to conditionally render all of those to prevent them from showing up in the builds under Optimize. I do worry if it'll bloat the UI too much though...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs more thought Needs more thought to flesh out the solution
Projects
None yet
Development

No branches or pull requests

2 participants