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

Fix Europa theme godroll indicator color and add godroll/archived to item preview on settings page #10673

Merged
merged 3 commits into from
Aug 5, 2024

Conversation

FlaminSarge
Copy link
Contributor

@FlaminSarge FlaminSarge commented Aug 4, 2024

Fixes #10640

Only including two mobile screenshots, for Default and Europa themes, as it's a bit redundant to have all of them.

Screenshots: Screenshot 2024-08-04 at 3 33 12 PM Screenshot 2024-08-04 at 3 36 04 PM Screenshot 2024-08-04 at 3 33 24 PM Screenshot 2024-08-04 at 3 34 09 PM Screenshot 2024-08-04 at 3 33 16 PM Screenshot 2024-08-04 at 3 33 20 PM Screenshot 2024-08-04 at 3 33 29 PM Screenshot 2024-08-04 at 3 33 33 PM Screenshot 2024-08-04 at 3 33 39 PM Screenshot 2024-08-04 at 3 33 44 PM

@FlaminSarge
Copy link
Contributor Author

I may need some input from non-colorblind folks on the visibility of the thumbs-down on some themes.

@lowPolySkeleton
Copy link
Member

lowPolySkeleton commented Aug 4, 2024

if you're going to display archived as an example it should probably be greyed out by default as archived items are. Otherwise I see no reason to have all the tags as examples archived could make sense... If it was representative but it's not in this example.

thumbs a good ideas for the preview, new item dot never changes so no real need for that either, we could fix the crafted icons in this while we are at it too, it's pretty invisible on some themes.

@FlaminSarge
Copy link
Contributor Author

I should also add mobile screenshots.

should probably be greyed out by default

Oh you're totally right, I forgot about that. Also doubles as not-matching current search query too, I think.

no reason to have all the tags as examples

I figured since we're including 4 items for thumbs up/thumbs down + masterwork/not, might as well have different tags on them. Wondering if I should also include searchHidden=true examples for thumbs up/thumbs down/masterwork, but that may be too many.

fix the crafted icons

We can merge #10670 and add both enhanced+crafted to this PR.

Copy link
Contributor

@bhollis bhollis left a comment

Choose a reason for hiding this comment

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

I'm not sure of the overall value of showing previews of all of these different weapons tiles - thumbs-down especially is not in any wishlist a user is likely to come across.

The idea behind the preview icons was solely to show the effect of specific settings - the "show new items" setting, and the "icon size" setting. We don't actually need more examples beyond that IMO.

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Aug 5, 2024

effect of specific settings

I agree that thumbs-down is a case that will rarely come up, but I do feel that Theme should be added to the list of the settings that it's meant to preview (changing theme then tabbing back over to Inventory to see how it looks would be really bad UX).
Thumbs-up and searchHidden would be good if we let Theme be one of the settings this previews. Enhanced/Crafted icon may also be useful but that's more tentative.

I'll remove the thumbs-down cases, but how do we feel about the others for the sake of Theme previewing?

@FlaminSarge
Copy link
Contributor Author

Not updating all the screenshots for the removal of badRoll but:
Screenshot 2024-08-05 at 1 40 02 AM

@FlaminSarge FlaminSarge changed the title Fix Europa theme godroll indicator color and add more examples for god/bad rolls when previewing themes Fix Europa theme godroll indicator color and add godroll/archived to item preview on settings page Aug 5, 2024
@bhollis bhollis merged commit c6d4d32 into DestinyItemManager:master Aug 5, 2024
6 checks passed
@FlaminSarge FlaminSarge deleted the themes branch August 6, 2024 01:09
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.

Europa Theme un-masterworked weapons "thumbs up" not visible; blends with white icon color
3 participants