-
-
Notifications
You must be signed in to change notification settings - Fork 87
feat: optimize the treemap view #1428
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for rsdoctor ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces significant optimizations and UX improvements to the TreeMap visualization component, bumping package versions to 1.3.13-beta.0 across the monorepo.
- Complete UI redesign with a collapsible sidebar for better space utilization
- New features including fullscreen mode, in-place search functionality, and multiple size type views (stat, parsed, gzipped)
- Enhanced visual styling with updated color palettes and improved tooltip formatting
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/webpack-plugin/package.json | Version bump to 1.3.13-beta.0 |
| packages/utils/package.json | Version bump to 1.3.13-beta.0 |
| packages/types/package.json | Version bump to 1.3.13-beta.0 |
| packages/sdk/package.json | Version bump to 1.3.13-beta.0 |
| packages/rspack-plugin/package.json | Version bump to 1.3.13-beta.0 |
| packages/graph/package.json | Version bump to 1.3.13-beta.0 |
| packages/document/package.json | Version bump to 1.3.13-beta.0 |
| packages/core/package.json | Version bump to 1.3.13-beta.0 |
| packages/components/src/components/Charts/treemap.module.scss | Complete CSS rewrite with sidebar layout, fullscreen support, and search results styling |
| packages/components/src/components/Charts/constants.ts | Updated color palette with new Ant Design colors and adjusted transparency values |
| packages/components/src/components/Charts/TreeMap.tsx | Major refactoring adding fullscreen, search, sidebar toggle, improved tooltips, and node highlighting features |
| packages/components/package.json | Version bump to 1.3.13-beta.0 |
| packages/client/package.json | Version bump to 1.3.13-beta.0 |
| packages/cli/package.json | Version bump to 1.3.13-beta.0 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div | ||
| key={index} | ||
| className={Styles['search-result-item']} | ||
| onClick={() => handleSearchResultClick(result.nodeId)} | ||
| title={result.path} | ||
| > | ||
| {displayPath || result.path} | ||
| </div> | ||
| ); |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Search result items are not keyboard accessible. These divs with onClick handlers should either be buttons or have proper keyboard accessibility attributes (role="button", tabIndex={0}, and keyboard event handlers for Enter/Space keys) to ensure users can navigate and select results using only the keyboard.
| chartRef.current; | ||
| } | ||
| } | ||
| }, [forwardedRef, chartRef.current]); |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The useEffect dependency array includes chartRef.current, which is a ref value. React refs don't trigger re-renders when their .current property changes, so including it in the dependency array is ineffective and could be misleading. The effect will only run when forwardedRef changes, not when chartRef.current changes. Consider removing chartRef.current from the dependency array.
| }, [forwardedRef, chartRef.current]); | |
| }, [forwardedRef]); |
| containerRef.current | ||
| .requestFullscreen() | ||
| .then(() => setIsFullscreen(true)) | ||
| .catch((err) => console.error('Failed to enter fullscreen:', err)); |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing requestFullscreen() directly may cause errors in browsers that don't support it or require vendor prefixes. Consider checking for browser support or providing a fallback:
if (containerRef.current?.requestFullscreen) {
containerRef.current.requestFullscreen()...
} else if ((containerRef.current as any)?.webkitRequestFullscreen) {
(containerRef.current as any).webkitRequestFullscreen();
}| containerRef.current | |
| .requestFullscreen() | |
| .then(() => setIsFullscreen(true)) | |
| .catch((err) => console.error('Failed to enter fullscreen:', err)); | |
| const el = containerRef.current as any; | |
| if (el.requestFullscreen) { | |
| el | |
| .requestFullscreen() | |
| .then(() => setIsFullscreen(true)) | |
| .catch((err: any) => console.error('Failed to enter fullscreen:', err)); | |
| } else if (el.webkitRequestFullscreen) { | |
| try { | |
| el.webkitRequestFullscreen(); | |
| setIsFullscreen(true); | |
| } catch (err) { | |
| console.error('Failed to enter fullscreen (webkit):', err); | |
| } | |
| } else if (el.mozRequestFullScreen) { | |
| try { | |
| el.mozRequestFullScreen(); | |
| setIsFullscreen(true); | |
| } catch (err) { | |
| console.error('Failed to enter fullscreen (moz):', err); | |
| } | |
| } else if (el.msRequestFullscreen) { | |
| try { | |
| el.msRequestFullscreen(); | |
| setIsFullscreen(true); | |
| } catch (err) { | |
| console.error('Failed to enter fullscreen (ms):', err); | |
| } | |
| } else { | |
| console.error('Fullscreen API is not supported in this browser.'); | |
| } |
| .sidebar-toggle { | ||
| position: absolute; | ||
| top: 10px; | ||
| right: 10px; | ||
| z-index: 20; | ||
| background: white; | ||
| border: 1px solid #ccc; | ||
| border-radius: 4px; | ||
| padding: 5px; | ||
| cursor: pointer; | ||
| transition: right 0.3s ease; | ||
|
|
||
| .statistics-space { | ||
| width: 100%; | ||
| } | ||
| &.collapsed { | ||
| right: -40px; |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The sidebar toggle button is positioned with a fixed offset (right: 10px and right: -40px when collapsed), which may cause issues on smaller screens or when the sidebar width changes. Consider using a more responsive approach or making these values configurable.
| <div | ||
| className={`${Styles['sidebar-toggle']} ${collapsed ? Styles.collapsed : ''}`} | ||
| onClick={() => setCollapsed(!collapsed)} | ||
| > | ||
| <div className={`checkbox-container ${collapsed ? 'collapsed' : ''}`}> | ||
| {collapsed ? <RightOutlined /> : <LeftOutlined />} | ||
| </div> |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sidebar toggle button is not keyboard accessible. While it has an onClick handler, it's a div element without proper keyboard navigation support. Consider:
- Using a
<button>element instead - Adding
role="button"andtabIndex={0} - Adding keyboard event handlers for Enter/Space keys
This ensures users who rely on keyboard navigation can toggle the sidebar.
| _rect: any, | ||
| size: any, | ||
| ) { | ||
| var obj = { top: pos[1] + 10 }; |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The var keyword is deprecated in modern JavaScript. Use const or let instead for better scoping and readability.
| var obj = { top: pos[1] + 10 }; | |
| const obj = { top: pos[1] + 10 }; |
| seriesIndex: 0, | ||
| name: nodeInfo.name, | ||
| }); | ||
| } catch (e) {} |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Empty catch blocks silently swallow errors, making debugging difficult. Consider logging the error or adding a comment explaining why the error can be safely ignored.
| } catch (e) {} | |
| } catch (e) { | |
| console.error( | |
| 'Failed to highlight node with name:', | |
| nodeInfo.name, | |
| e, | |
| ); | |
| } |
|
commit message should starts with |
Summary
optimize the treemap view
Related Links