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

Better display for exotic class item perks, remove stats from class items in itemfeed #10557

Merged
merged 2 commits into from
Jun 25, 2024

Conversation

FlaminSarge
Copy link
Contributor

@FlaminSarge FlaminSarge commented Jun 16, 2024

Makes exotic class items use the intrinsicRow display instead of normal socket UI. It also treats exotic class items' intrinsic perks as non-reusable so that they don't get highlighted as selectable perks in the UI.

This also removes the stat display from class items in the item feed.

Desktop: Screenshot 2024-06-16 at 4 15 24 AM Screenshot 2024-06-16 at 4 21 44 AM Screenshot 2024-06-16 at 4 15 13 AM Screenshot 2024-06-18 at 2 00 14 PM
Mobile: Screenshot 2024-06-19 at 2 11 54 AM Screenshot 2024-06-19 at 2 12 06 AM Screenshot 2024-06-19 at 2 11 30 AM Screenshot 2024-06-19 at 2 12 42 AM

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jun 16, 2024

Something in this implementation is clobbering perks in d2-stores' snapshot test, so investigating that before I move out of draft.

In the meantime please let me know of any esoteric cases I should test for this.

@FlaminSarge
Copy link
Contributor Author

It seems like the isReusable hack can be omitted if https://github.com/DestinyItemManager/DIM/tree/archetype-presstip goes in, though I'm not sure which solution is the right one to go for.

@FlaminSarge
Copy link
Contributor Author

Do class items really need stat display on the item feed?

@bhollis
Copy link
Contributor

bhollis commented Jun 18, 2024

Nope

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jun 18, 2024

Removed the stats from class items; can revert if needed.
Screenshot 2024-06-18 at 2 00 14 PM

@FlaminSarge FlaminSarge changed the title Initial implementation of better display of exotic class item perks Better display for exotic class item perks, remove stats from class items in itemfeed Jun 19, 2024
@bhollis bhollis requested a review from robojumper June 19, 2024 19:11
@bhollis
Copy link
Contributor

bhollis commented Jun 19, 2024

@robojumper you've got the most experience with the socket stuff - would love your opinion on this.

@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jun 19, 2024

I've got an alternate way of handling this master...FlaminSarge:DIM:exoticclass2

I've swapped the contents of master...FlaminSarge:DIM:exoticclass2 and master...FlaminSarge:DIM:exoticclass as the other implementation is much cleaner and doesn't touch nearly as many places due to omitting the 'multiple intrinsics' change.

UI remains consistent with the other, and we don't run into the issues associated with changing intrinsicSocket -> intrinsicSockets.

…perks on item feed

Remove stats from class items in item feed
src/app/item-popup/ItemSocketsGeneral.tsx Outdated Show resolved Hide resolved
src/app/item-popup/ItemSocketsGeneral.tsx Outdated Show resolved Hide resolved
src/app/item-popup/ItemSocketsGeneral.tsx Outdated Show resolved Hide resolved
src/app/item-feed/Highlights.tsx Outdated Show resolved Hide resolved
move extra intrinsic parsing to util method
fix casting/filtering
make intrinsic socket handling allow multiple plugOptions for armor to align it with weapon impl, just in case
@FlaminSarge
Copy link
Contributor Author

FlaminSarge commented Jun 25, 2024

As a consequence of updating it to show multiperk sockets in feed (to mirror impl of previous weapon PR for consistency), the spacing has changed by 1px for each perk column:
Screenshot 2024-06-24 at 6 33 46 PM

It was either do this, or add some strange styles to try to get around the fact that armor divs were less nested than weapon divs.

@nev-r nev-r merged commit 84d15e1 into DestinyItemManager:master Jun 25, 2024
6 checks passed
@FlaminSarge FlaminSarge deleted the exoticclass branch June 25, 2024 10:02
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.

3 participants