frzyc / genshin-optimizer

An Artifact optimizer for Genshin Impact.
https://frzyc.github.io/genshin-optimizer/
MIT License
749 stars 218 forks source link

Use Equipment confirmation modal for equipping arti/weapons #2054

Open frzyc opened 2 months ago

frzyc commented 2 months ago

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 commented 2 months ago

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.

arquelion commented 2 months ago

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...