CharacterSelectionModal refactoring and followup changes for single selection

[arquelion]

Describe your changes

CharacterSelectionModal and CharacterMultiSelectionModal are now in the same file and refactored as follows:

• CharacterSingleSelectionModal and CharacterMultiSelectionModal are the exported components utilizing the rest of the components in CharacterSelectionModal.tsx to form the modal's structure and interactivity. These modals no longer fetch loadoutData themselves and instead have them passed in as props when necessary.
• CharacterSelectionModalBase contains most of the structural and filter/search/sort code. CharacterSingleSelectionModal and CharacterMultiSelectionModal are responsible for passing in the vast majority of functions, character key lists, etc. that this component needs (listed in CharacterSelectionModalBaseProps and FilterSearchSortProps).
• SelectionCard has had CustomTooltip, CardThemed, and the IconButton for favorites moved into new components SingleSelectCardWrapper and MultiSelectCardWrapper. SingleSelectCardWrapper also contains the new functionality for animating the outline on the slot currently being selected, and MultiSelectCardWrapper contains the functionality added in PR 2440 for outlining currently selected teammates and indicating their team slots for multi selection.
• CharacterMultiSelectionModal.tsx deleted as a result of these changes and all current call sites changed to use single or multi select as necessary

Single character selection in CharacterSelectionModal.tsx:

• Currently selected teammates (when relevant) are now pinned to the front of the selection list and bypass filter/sort/search
• Instead of a chip indicating the character's current team slot, single selection simply outlines currently selected characters but flashes the outline for the slot being changed. (Outlines transition between gold and the background color of the modal as this is significantly more visible than flashing white.)

Issue or discord link

• Follow-up work for Add option to select multiple characters at a time when setting up a team #2440

Testing/validation

Team single select outlines:

team-single-select-outlines.mp4

I've manually checked all interactions I can think to confirm the refactor has not broken anything.

Checklist before requesting a review (leave this PR as draft if any part of this list is not done.)

• I have commented my code in hard-to understand areas.
• I have made corresponding changes to README or wiki.
• For front-end changes, I have updated the corresponding English translations.
• I have run yarn run mini-ci locally to validate format and lint.
• If I have added a new library or app, I have updated the deployment scripts to ignore changes as needed

[github-actions]

[frontend] [Tue Oct 1 23:08:31 UTC 2024] - Deployed e7206b7 to https://genshin-optimizer-prs.github.io/pr/2473/frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Sun Oct 6 21:21:44 UTC 2024] - Deployed a31f408 to https://genshin-optimizer-prs.github.io/pr/2473/frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Mon Oct 7 09:54:56 UTC 2024] - Deployed 7cfdce1 to https://genshin-optimizer-prs.github.io/pr/2473/frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Tue Oct 8 23:05:06 UTC 2024] - Deployed c34083e to https://genshin-optimizer-prs.github.io/pr/2473/frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Tue Oct 8 23:05:38 UTC 2024] - Deployed 096c46a to https://genshin-optimizer-prs.github.io/pr/2473/frontend (Takes 3-5 minutes after this completes to be available)

[frontend] [Wed Oct 9 00:16:55 UTC 2024] - Deployed ffeb921 to https://genshin-optimizer-prs.github.io/pr/2473/frontend (Takes 3-5 minutes after this completes to be available)

[Wed Oct 9 00:52:44 UTC 2024] - Deleted deployment

[frzyc]

Before you go too far with this, I do not think it is a good idea to merge the logic of 2 elements with two different usecases, since that makes the core code path more complicated and harder to maintain. If you want to reduce duplication of code, hoist out the common elements into their separate function/element.

[arquelion]

Honestly I was kind of wondering whether to split this back up into a regular CharacterSelectionModal and a TeamCharacterSelectionModal. With the changes to single select to display all currently selected team members etc., single and multi select where teams are concerned actually share almost everything, just a couple small differences in whether to show the team slot number and whether to flash the outline for the currently selected slot. It does feel like there's just enough different in the code between the Teams use case and the Characters page use case though.

[frzyc]

Yeah I understand your concern, In this case, I think you should still have some division of components:

SingleSelect(4 selected, currentlySelected, onSelect)

• CardWrapper(flashing/selected +index)
  • characterCard

MultiSelect(4 selected, onSelectTeam)

store 4 selected state locally(for quick selection + save)

• CardWrapper(show border+index)
  • characterCard

This should make sure that each component only does one thing with one unique state each, instead of a super component that does 2 things in different states depending on usecase

[arquelion]

Trying one more approach based on your suggestion before I throw in the towel and deal with all the duplicated code: I split out some of the shared modal code out into a CharacterSelectionModalBase that both the single and multi select versions use and also split out some of the SelectionCard code into the two wrapper components SingleSelectCardWrapper and MultiSelectCardWrapper which also contain their respective outlining/animation/slot indicators (so SelectionCard really is just the card now).

Also I'm not sure what the types on the props for the onChange____ functions should be or if I should just leave them as implicitly any like some of the code in other locations do. I took the types from mousing over the locations where these functions are passed as props.

[arquelion]

@frzyc I'll go through it again to see what could be cleaned up, but otherwise good for a first round of review

[frzyc]

@arquelion This looks good to me. I like the organization and the separation of concerns here.

[arquelion]

Okay, I went through and added some usage comments but otherwise didn't find anything else that I felt needed comments that didn't already have them. Also added a quick recording of the new additions to single select and manually checked as many interactions as I could think of. (Second pair of eyes double-checking all the possible use cases would be nice though.)

[frzyc]

LGTM, Thanks for iterating on this.