-
Notifications
You must be signed in to change notification settings - Fork 228
feat: make control panel vertical and fix unnecessary button focus behaviors #2261
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
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
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. |
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 zoom controls panel in the WDS topology view, changing it from a horizontal layout positioned on the left to a vertical layout positioned on the right side. The changes improve visual hierarchy and add responsive scaling based on viewport height, while also removing focus outlines from buttons.
Key changes:
- Vertical layout with right-side positioning for better space utilization
- Responsive scaling system that adapts control sizes based on viewport height
- Removal of focus outlines on buttons (
:focusand:focus-visible)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/cc @clubanderson PTAL |
|
@khushiiagrawal: GitHub didn't allow me to request PR reviews from the following users: PTAL. Note that only kubestellar members and repo collaborators can review this PR, and authors cannot review their own PRs. DetailsIn response to this:
Instructions 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. |
|
PTAL @kunal-511 @btwshivam |
|
/ok-to-test |
|
the CI's are failing |
@Per0x1de-1337 I think this problem is not because of the PR, it's the issue with Helm Chart. |
|
@khushiiagrawal Too much lag and rendering. make it smooth. also test same with WECS |
@btwshivam i've made the rendering smoother and it works for WECS also. Screen.Recording.2025-11-29.at.7.07.14.PM.mov |
|
@btwshivam can you advise on this? |
|
The whole screen is being zoomed in and out fix it. set refernce for treeview screen only not whole window. |
|
The useEffect that subscribes to rf.on('move', ...) and the RAF fallback uses many external values (rf, getViewport, getZoom, snapToStep, setZoom). dont left it empty |
|
i mean set minimum dependency.. also use usememo whereever the calculation is being done |
|
In the RAF fallback branch you set rafId and then requestAnimationFrame repeatedly. eensure the cleanup properly cancels rafId when the component unmounts or when branch conditions change. i mean this can memory leak |
|
@btwshivam PTAL. |
|
/ok-to-test |
Screen.Recording.2025-12-09.at.2.29.37.AM.mov |
|
@btwshivam is this ready? |
|
@khushiiagrawal Again whole window getting zoomed in out |
|
told you to set reference |
@btwshivam i have set the reference for the for treeview screen as you asked |
@btwshivam isn't it because i am increasing and decreasing the screen size ? |
hmm .. show me how it is behaving on zoom in out and why you are resizing the screen to show zoom in out |
i was showing the control panel behavior as it is responsive, i'll attach with the treeview zoom also |
Screen.Recording.2025-12-09.at.10.35.37.PM.mov |
|
looks good but have to figureout something for less then 60 zoom. it look consaliated |
|
can you try one thing for me.. try passing a statement if zoom out is less then 60 then double the horizontal sepration and vertical sepration also.. use memo for recalculation |
its done. PTAL /cc @btwshivam Before Screen.Recording.2025-12-09.at.11.46.23.PM.movAfter Screen.Recording.2025-12-09.at.11.55.50.PM.mov |
e5caa18 to
245ea6a
Compare
|
Much better right.. |
yeah, right! |
|
no you are redifining node and rank sep again use existing... it will create problem for wecs and if someone change from canvas flow then also |
|
ok then do one thing revet last commit -- 1 thing is wecs and wds node and ranksep should be same |
Okay @btwshivam , got your point. Will do it. |
150afb8 to
245ea6a
Compare
|
@btwshivam , I have checked everything and now the branch is at the commit with you were expecting. |
|
@btwshivam is this ready to go? |
|
yes merge it next will fix in next pR |
|
LGTM label has been added. DetailsGit tree hash: 817ce3e05cfb4c5835814bcfa5c2c4ef680776a6 |

Description
This PR refactors the zoom controls panel in the WDS topology view to improve UX and accessibility. The main changes include:
Related Issue
Fixes #2248
Changes Made
ZoomControls.tsxto use vertical layout (flexDirection: 'column')outline: 'none'for:focusand:focus-visible)Checklist
Please ensure the following before submitting your PR:
Screenshots or Logs (if applicable)
Screen.Recording.2025-11-20.at.11.38.16.PM.mov
Additional Notes