-
Notifications
You must be signed in to change notification settings - Fork 221
refactor(treeView): implement fixed node styling #2295
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
Conversation
Signed-off-by: Khushi Agrawal <[email protected]>
|
Hi @khushiiagrawal. Thanks for your PR. I'm waiting for a kubestellar member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
@btwshivam PTAL. |
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 refactors the TreeView component to use fixed node styling instead of dynamic zoom-based scaling. The changes remove dependencies on the zoom store and simplify the codebase by establishing consistent, hardcoded dimensions (146px × 30px) for tree view nodes.
Key changes:
- Removed zoom store dependencies from TreeView components and replaced dynamic scaling calculations with fixed dimensions
- Updated layout engine to use fixed node dimensions instead of zoom-based scaling factors
- Simplified NodeLabel component by replacing dynamic width/height calculations with fixed pixel values
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
frontend/src/components/treeView/TreeViewNodes.tsx |
Removed zoom store import and replaced getScaledNodeStyle() calls with a fixedNodeStyle object containing hardcoded dimensions; cleaned up dependency arrays |
frontend/src/components/treeView/hooks/useTreeViewData.ts |
Replaced zoom-based scaling calculations in layout engine with fixed NODE_WIDTH (146) and NODE_HEIGHT (30) constants; removed zoom store import |
frontend/src/components/wds_topology/NodeLabel.tsx |
Replaced dynamic min/max width/height calculations based on iconSize with fixed pixel values; removed iconSize from dependency array |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Khushi Agrawal <[email protected]>
|
/ok-to-test |
|
good Job that looks smooth @khushiiagrawal |
|
Font size of 6px is extremely small This is barely readable on most screens. make it 10-12px. |
|
just a suggestion before merge please check is there now a mismatch between React Flow's zoom and node sizes? |
…etter readability Signed-off-by: Khushi Agrawal <[email protected]>
@btwshivam I checked the React Flow zoom and fixed node sizes. Thanks! |
|
Okay.. Have to tested with 12px font? ensuure nothing breaks.. next we need to fix edegs |
|
/ok-to-test |
|
LGTM label has been added. DetailsGit tree hash: 1c222ccf23f88809d596222a5026881fbf3b079d |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: btwshivam The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
This PR refactors the TreeView component to implement fixed node styling, replacing the previous dynamic zoom-based scaling system. The changes simplify the codebase by removing zoom-dependent calculations and establishing consistent, fixed dimensions for tree view nodes across all zoom levels.
Related Issue
Fixes #2276
Changes Made
useZoomStoreand replaced dynamicgetScaledNodeStyle()calls with fixed node styling constantscurrentZoomandgetScaledNodeStylereferencesChecklist
Please ensure the following before submitting your PR:
Screenshots or Logs (if applicable)
Untitled.mov
Additional Notes