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

Replace red background of cannotRoll perks with dotted circle outline #10679

Merged
merged 6 commits into from
Aug 11, 2024

Conversation

FlaminSarge
Copy link
Contributor

@FlaminSarge FlaminSarge commented Aug 8, 2024

Fixes #10550

This is more readable than the slightly different shades of red used for selected/unselected cannotRoll perks.

Alternatives considered: small faExclamationCircle in the bottom right/left of the icon. This ended up clashing with the thumbs up icon if on the bottom right, and the enhancement arrow if on the bottom left.

Screenshots:

image
image
image
image
image
image

Updated screenshots for later changes in this PR:
Realigned dashes so gaps are in cardinal directions: Screenshot 2024-08-08 at 3 53 55 AM

Added dashed circle to tooltip text: Screenshot 2024-08-10 at 3 27 46 AM

@FlaminSarge
Copy link
Contributor Author

Adjusted dashoffset:
Screenshot 2024-08-08 at 3 53 55 AM

@bhollis
Copy link
Contributor

bhollis commented Aug 9, 2024

Nice. The one bit that I'm not entirely happy with is that this doesn't really communicate "can't roll" to me - it seems more like "not unlocked" or something like that. I'm not sure what the solution is. The perk is there, it's just no longer unlockable for new rolls...

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Aug 9, 2024

Nice. The one bit that I'm not entirely happy with is that this doesn't really communicate "can't roll" to me - it seems more like "not unlocked" or something like that. I'm not sure what the solution is. The perk is there, it's just no longer unlockable for new rolls...

Thoughts on this? It's a little bit awkward but directly ties the concept of the dashed icon to the tooltip warning about cannotRoll.

Old preview: Screenshot 2024-08-09 at 3 19 02 PM

@bhollis
Copy link
Contributor

bhollis commented Aug 10, 2024

I'm not sure that's quite it either. Maybe the dashed border is fine on its own.

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Aug 10, 2024

I'm not sure that's quite it either. Maybe the dashed border is fine on its own.

One last iteration, and I think this one looks slick. I'm not sure if this is the right way to implement it (with an empty span), though.

Screenshot 2024-08-10 at 3 27 46 AM

@bhollis
Copy link
Contributor

bhollis commented Aug 10, 2024

Yeah that's quite nice.

@FlaminSarge FlaminSarge merged commit ca6da4c into DestinyItemManager:master Aug 11, 2024
6 checks passed
@FlaminSarge FlaminSarge deleted the cant-roll branch August 11, 2024 22:45
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.

enhanced (gold) but "cannot currently roll" (red) perks are hard to tell if they're selected (normally blue)
2 participants