[DevTools] Avoid scrollbars in Suspense breadcrumbs#468
Conversation
everettbu
added
CLA Signed
React Core Team
6f44da2 to
d538e85
Compare
Greptile OverviewGreptile SummaryThis PR updates the Suspense tab breadcrumbs UI to avoid horizontal scrollbars in the one-line header area by switching to an overflow-aware presentation (similar to the Components tab Owner stack): when the breadcrumb list would overflow, it collapses into a menu + navigation controls. Implementation-wise, the old Confidence Score: 3/5
Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| type SuspenseBreadcrumbsDropdownProps = { | ||
| lineage: $ReadOnlyArray<SuspenseNode['id']>, | ||
| selectedIndex: number, | ||
| selectElement: (id: SuspenseNode['id']) => void, | ||
| }; |
There was a problem hiding this comment.
Broken Flow props type
SuspenseBreadcrumbsDropdownProps declares selectedIndex, but SuspenseBreadcrumbsDropdown is invoked without it (SuspenseBreadcrumbs.js:146-149) and the function signature also omits it. This will fail Flow checking (and makes the type misleading).
| type SuspenseBreadcrumbsDropdownProps = { | |
| lineage: $ReadOnlyArray<SuspenseNode['id']>, | |
| selectedIndex: number, | |
| selectElement: (id: SuspenseNode['id']) => void, | |
| }; | |
| type SuspenseBreadcrumbsDropdownProps = { | |
| lineage: $ReadOnlyArray<SuspenseNode['id']>, | |
| - selectedIndex: number, | |
| selectElement: (id: SuspenseNode['id']) => void, | |
| }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js
Line: 177:181
Comment:
**Broken Flow props type**
`SuspenseBreadcrumbsDropdownProps` declares `selectedIndex`, but `SuspenseBreadcrumbsDropdown` is invoked without it (`SuspenseBreadcrumbs.js:146-149`) and the function signature also omits it. This will fail Flow checking (and makes the type misleading).
```suggestion
type SuspenseBreadcrumbsDropdownProps = {
lineage: $ReadOnlyArray<SuspenseNode['id']>,
- selectedIndex: number,
selectElement: (id: SuspenseNode['id']) => void,
};
```
How can I resolve this? If you propose a fix, please make it concise.| <MenuItem | ||
| key={suspenseNodeID} | ||
| className={`${styles.Component}`} | ||
| onSelect={selectElement.bind(null, suspenseNodeID)}> | ||
| {node === null ? 'Unknown' : node.name || 'Unknown'} |
There was a problem hiding this comment.
Missing CSS class
className={${styles.Component}} references styles.Component, but SuspenseBreadcrumbs.css doesn't define a .Component class, so this will end up as an empty class and menu items won't get the intended styling.
If this was meant to share the menu item styling from OwnersStack, it needs a local class in SuspenseBreadcrumbs.css (or use an existing class name that actually exists in this module).
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js
Line: 193:197
Comment:
**Missing CSS class**
`className={`${styles.Component}`}` references `styles.Component`, but `SuspenseBreadcrumbs.css` doesn't define a `.Component` class, so this will end up as an empty class and menu items won't get the intended styling.
If this was meant to share the menu item styling from OwnersStack, it needs a local class in `SuspenseBreadcrumbs.css` (or use an existing class name that actually exists in this module).
How can I resolve this? If you propose a fix, please make it concise.| <Button | ||
| className={parent !== null ? undefined : styles.NotInStore} | ||
| onClick={parent !== null ? selectElement.bind(null, parentID) : null} | ||
| title={`Up to ${parent === null ? 'Unknown' : parent.name || 'Unknown'}`}> |
There was a problem hiding this comment.
Missing NotInStore style
styles.NotInStore is used when the parent node is missing (SuspenseBreadcrumbs.js:245), but there is no .NotInStore in SuspenseBreadcrumbs.css. As a result, the "disabled" styling won't apply (and the button will look interactive even though onClick is null).
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js
Line: 244:247
Comment:
**Missing NotInStore style**
`styles.NotInStore` is used when the parent node is missing (`SuspenseBreadcrumbs.js:245`), but there is no `.NotInStore` in `SuspenseBreadcrumbs.css`. As a result, the "disabled" styling won't apply (and the button will look interactive even though `onClick` is null).
How can I resolve this? If you propose a fix, please make it concise.| const ResizeObserver = container.ownerDocument.defaultView.ResizeObserver; | ||
| const observer = new ResizeObserver(() => { |
There was a problem hiding this comment.
ResizeObserver may be undefined
This assumes container.ownerDocument.defaultView.ResizeObserver exists and then instantiates it unconditionally. In environments without ResizeObserver (or if it’s not exposed on that window), this will throw at runtime when breadcrumbs render in the non-overflowing state.
OwnersStack avoids this by only measuring without creating a new observer; here you probably want to guard ResizeObserver before new ResizeObserver(...).
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/react-devtools-shared/src/devtools/views/SuspenseTab/SuspenseBreadcrumbs.js
Line: 285:286
Comment:
**ResizeObserver may be undefined**
This assumes `container.ownerDocument.defaultView.ResizeObserver` exists and then instantiates it unconditionally. In environments without `ResizeObserver` (or if it’s not exposed on that window), this will throw at runtime when breadcrumbs render in the non-overflowing state.
OwnersStack avoids this by only measuring without creating a new observer; here you probably want to guard `ResizeObserver` before `new ResizeObserver(...)`.
How can I resolve this? If you propose a fix, please make it concise.
|
Upstream PR was closed or merged. Code is synced via branch mirror. |
2 participants
Mirror of facebook/react#35700
Original author: eps1lon
Summary
Alternate to react/react#34914
The Suspense breadcrumbs are only a single line height high. Having a scrollbar in such a container looks bad since it takes up 50% of the space. MacOS also has this default where the scrollbar overlays the container on touch devices without a mouse attached (i.e. MacBook default).
Now we use the same UI we use in the Owner stack in the Components tab i.e. shrink to showing a single item and moving navigation into a menu.
How did you test this change?
Before:
After:
CleanShot.2026-02-05.at.14.59.44.mp4