feat(frontend): add title & desc mode to menu selector#1707
Merged
Conversation
Contributor
Reviewer's GuideExtends the MenuSelector component to support labels with separate title and description fields and updates game settings menus and i18n resources to use this new title/description display mode for certain options. File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
isTitleDescLabelruntime check relies ontypeof label === "object", which will also be true for React elements; consider using a more robust discriminator (e.g., a dedicated type field or explicitly excludingReact.isValidElement(label)) so ReactNodes are not misclassified as{ title, desc }. - In
renderButtonLabel,getLabelcan still return a non-string ReactNode for non-title/desc labels, but you then call.join(", ")in the multi-select case—normalizing these labels to strings (or handling non-string labels explicitly) would avoid potential runtime issues and inconsistent button text.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `isTitleDescLabel` runtime check relies on `typeof label === "object"`, which will also be true for React elements; consider using a more robust discriminator (e.g., a dedicated type field or explicitly excluding `React.isValidElement(label)`) so ReactNodes are not misclassified as `{ title, desc }`.
- In `renderButtonLabel`, `getLabel` can still return a non-string ReactNode for non-title/desc labels, but you then call `.join(", ")` in the multi-select case—normalizing these labels to strings (or handling non-string labels explicitly) would avoid potential runtime issues and inconsistent button text.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…djust java menu Co-authored-by: 3gf8jv4dv <3gf8jv4dv@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
This PR is a ..
Related Issues
close #1706
Description
Additional Context
Summary by Sourcery
Add support for title-and-description labels in the shared menu selector and apply it to advanced game settings menus.
New Features:
Enhancements: