Remove dead code from ListItemLink#2008
Open
GregorShear wants to merge 4 commits into
Open
Conversation
ListItemLink's only consumer (Navigation.tsx) passes just icon, title, and a string link. The isOpen, menuWidth, badgeContent, and tooltipDelay props were never passed by any consumer, so the conditionals they drove were dead: the menuWidth === NavWidths.FULL branch never ran, the tooltip title was always translatedTitle, enterDelay was always undefined, and the Badge could never render content. Since link is always a string, component is always RouterLink, to is always link, and the function-link/onClick path was dead too. Narrow link to string and icon to ReactElement (all consumers pass a single JSX icon element), and render ListItemIcon unconditionally now that icon is always provided.
MuiDrawer uses variant="permanent", which renders only the docked Paper — never the Modal or Slide transition. So open, ModalProps, and onClose were inert: open feeds only the Slide transition, while ModalProps and onClose are Modal props. Removing onClose leaves closeNavigation unused, so drop it too. The open prop into the component stays; it still drives the collapse-arrow rotation, label visibility, and tooltip delay.
Replace react-intl message lookups with the literal English copy in ListItemLink, ModeSwitch, and Navigation. ListItemLink's title prop now carries the display string directly (Navigation passes the literals: Welcome, Sources, Collections, Destinations, Admin) rather than a route-title message id resolved via useIntl. Aria-labels, the collapse label, and the color-mode label are inlined too. Removes navigation.toggle.ariaLabel, navigation.collapse, and modeSwitch.label from the en-US Navigation messages, which are now orphaned. Route-title and terms.* keys are left intact since they are used elsewhere (routes, page titles). PageTitle keeps react-intl: its header is a message id read from the TopBar store and set via usePageTitle across many pages, so inlining it would require changing that contract app-wide, beyond the nav/topbar scope.
Revert the inlined nav labels back to authenticatedRoutes.X.title and restore ListItemLink's useIntl lookup. These labels source their copy from routes.ts route titles (still react-intl message ids), so inlining them here duplicated copy that should stay single-sourced until the deferred routes.ts -> English refactor lands. Standalone nav copy not coupled to routes.ts (aria-labels, collapse label, color-mode label) stays inlined.
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
1 participant
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.
Simplify and delete dead code from navigation components. Remove react-intl