Vault weapon grouping/separators

[JakeSidSmith]

This is a work in progress. At the bottom is a checklist of items that still need to be completed/considered.

Fixes #7206
Fixes #3613

This PR adds 2 1 option (dropdown) to the settings page, one allowing the user to define a property by which to group vault items, and another that allows them to tweak how the groupings are displayed. I've limited the properties by which items can be grouped as many were not appropriate for displaying (e.g. power level and acquired date end up with far too many groups).

I've opted to use dropdown for these options as opposed to adding checkboxes or other inputs to the sort options (as mentioned in some issues), as this keep the UI a lot cleaner and doesn't involve the need to enforce only a single grouping (which would be required if the option was added to custom sort items).

In any grouping display setting the groups will be prefixed with an icon (yet to be implemented), much like how class armor has the class icon. In the "Separated" view groups are all displayed on separate rows with horizontal lines between each, whereas the "Best Fit" view will display groups without the horizontal lines, and will include multiple groups in a single row if there is enough space.

All groups except for rarity are prefixed with an icon, much like the class specific armor groups.

See comments for discussion and updated screenshots.

Screenshots

Grouping with "Best Fit" Grouping with "Separated"

To consider:

• Is it worth adding any of these groupings to the armor/inventory items?
• How can this be extended to allow custom grouping (e.g. grouping by pvp/pve that are mentioned in several issues that I now cannot find)?

TODO:

• Extract conditional grouping within StoreBucket to its own component - only groups weapons
• Add doc comments to new utils/selectors
• Add tests for new utils/selectors
• Add icons for each grouping type (including an "unknown"/"ungrouped" icon) - removed icons/tiles
• Add types to DIM API types?
• Remove grouping from armor/inventory items? - only groups weapons
• Improve grouping display of armor/inventory items? - only groups weapons

[bhollis]

It's a nice start! Code looks like it's going in the right direction. Some stuff to think about:

• Best fit doesn't look super good - lots of half-filled lines. Perhaps it's better to do that one the way we did armor - flatten out the lists and just insert the group separators in the grid. The downside is you can have awkward situations like the separator tile falling at the end of a row. Still, "best fit" is awkward enough right now that it definitely shouldn't be the default, and probably shouldn't be an option as is.
• For some groupings, the order of the group is somewhat arbitrary. Weapon type, which IMO is the most interesting (maybe the only interesting) grouping, uses an ordering that I probably wouldn't have if I had chosen the order by hand.
• I'm not sure this brings much value to armor. Most of the groupings don't even apply to armor. I'd remove it.
• I'm repeating myself, but everything other than weapon type and ammo type are trivially distinguishable if you simply sort by that property first (e.g. tag or rarity is very clear if it's your top sort). Putting them on different lines doesn't offer much.
• The separated view isn't bad - it probably doesn't need the line if it has a tile that explains it. But I wonder if they even need that tile - maybe it's super obvious, except for maybe weapon type and ammo type.

[JakeSidSmith]

Thank you. 😊

• Best fit doesn't look super good - lots of half-filled lines. Perhaps it's better to do that one the way we did armor - flatten out the lists and just insert the group separators in the grid. The downside is you can have awkward situations like the separator tile falling at the end of a row. Still, "best fit" is awkward enough right now that it definitely shouldn't be the default, and probably shouldn't be an option as is.

I'm happy to remove the best fit view. I'm not a fan either.

• For some groupings, the order of the group is somewhat arbitrary. Weapon type, which IMO is the most interesting (maybe the only interesting) grouping, uses an ordering that I probably wouldn't have if I had chosen the order by hand.

All the groupings are based on the existing sort options, so as to maintain consistency with the sorts. I believe weapon types are alphabetical?

• I'm not sure this brings much value to armor. Most of the groupings don't even apply to armor. I'd remove it.

Agreed.

• I'm repeating myself, but everything other than weapon type and ammo type are trivially distinguishable if you simply sort by that property first (e.g. tag or rarity is very clear if it's your top sort). Putting them on different lines doesn't offer much.

I agree that rarity is fairly obvious (due to the colorings), but I personally wanted to implement this feature to distinguish between the tags (failing a way to manually group items). As all the tag icons are very small and the same color it's hard to see where one ends and another begins.

• The separated view isn't bad - it probably doesn't need the line if it has a tile that explains it. But I wonder if they even need that tile - maybe it's super obvious, except for maybe weapon type and ammo type.

The tile was basically a result of the best fit view. More code and harder to maintain if one view has icons and another didn't, or that one was flat and another nested. Happy to remove them if we're just going with the separated view.

[bhollis]

All the groupings are based on the existing sort options, so as to maintain consistency with the sorts. I believe weapon types are alphabetical?

Yeah, I don't use sort by weapon type, and the grouping makes it even more obvious that it's not a particularly useful sort. I'm not sure what would be more useful though - ordering them by ammo type is just including another sort.

As all the tag icons are very small and the same color it's hard to see where one ends and another begins.

That's interesting - I find their shapes quite distinctive, but I could see the argument.

[JakeSidSmith]

I was actually going to suggest (as a separate issue) that the tag icons each receive a unique color, but I understand that this is more work than it sounds due to the theming.

[bhollis]

They're also all the same color on purpose - that green is the only thing we can use that stands out against all the different item icons.

[JakeSidSmith]

They're also all the same color on purpose - that green is the only thing we can use that stands out against all the different item icons.

Ah, I had not considered that. 😕

[JakeSidSmith]

I have now...

• Removed armor grouping
• Renamed the groupings as they are specific to weapons
• Removed the best fit view (and display options)
• Removed tiles/icons rendered for each group
• Added comments to grouping utils
• Added tests for grouping selectors

I'm not sure if/how the API types are updated. Please advise on how to proceed. 😊

Updated screenshots:

[bhollis]

Looks good, definitely works as advertised. A couple things I noticed:

• Without an icon, it can be hard to understand what a category contains. Even for more obvious ones like damage type it takes a second to figure out. I think we really should have them, but it makes it hard to support as many options.
• This says it only affects weapons, but it clearly also groups my armor (at least in single character mode) - e.g. when sorting by type, FoTL masks are separated, when sorting by rarity exotics are separate. For some of the sorts (rarity, tag) that's actually nice, for some it doesn't mean much.
• We probably need to have custom / handcrafted orders for most of these. For example, sorting by tag puts my "archived" items first.
• I can sorta see why you'd choose most of these, but "Deepsight" doesn't have a lot of value.

API types are updated here: https://github.com/DestinyItemManager/dim-api/blob/master/api/shapes/settings.ts - it's OK to just update initial-settings, I move new settings to the API types after a while.

[JakeSidSmith]

I cannot update my branch from master due to a prettier error in src/index.html.

Tried to fix this myself, but seems prettier just doesn't like the lodash template syntax.

[JakeSidSmith]

Tag ordering has been improved, and have added icons for all groupings except rarity (as the colors are fairly obvious and I'm not sure there's a suitable icon for these).

Think I'm happy for this to ship now. 😁 Thoughts?

[bhollis]

We're so close, I can taste it! I love the way this looks and works now. A couple last bits:

1. The ammo icons are a touch small compared to the others.
2. In single character mode, no armor shows up at all.

[JakeSidSmith]

Adjusted icon sizes a bit - ammo and element are now larger.

When you say "single character mode", are you referring to collapsing the other characters with the left arrow next to "Vault"? e.g.

[bhollis]

That's my bad, I was testing a different character who doesn't actually have any items in the vault 😅

[JakeSidSmith]

@bhollis have made all suggested changes. 😁

[bhollis]

Great! Let's get this merged!

[bhollis]

We generally use this:

Suggested change

	export const vaultGroupTagOrder = itemTagList.map((tag) => tag.type).filter(isDefined);
	export const vaultGroupTagOrder = _.compact(itemTagList.map((tag) => tag.type));