Skip to content

frontend: expand crs on sidebar #3366

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

farodin91
Copy link
Contributor

@farodin91 farodin91 commented May 26, 2025

Idea to make CRs more accessible:

image

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels May 26, 2025
@k8s-ci-robot k8s-ci-robot requested review from sniok and vyncent-t May 26, 2025 20:22
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: farodin91
Once this PR has been reviewed and has the lgtm label, please assign sniok for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 26, 2025
@farodin91
Copy link
Contributor Author

@illume I would like to get feedback.

@farodin91
Copy link
Contributor Author

Some tests are expected to fail, until i known its going in the right direction.

@ashu8912
Copy link
Member

Hii @farodin91 thanks for these changes, I have tried the PR and would like to add a comment.

  1. Why do we need the CRs on the sidebar?? Is it because of the friction to go to the instance page, I ask this because having all the instances like this in the sidebar doesn't look very good.

@farodin91
Copy link
Contributor Author

@ashu8912 As long as there is no plugin, I think it makes the CRDs more usable. I had this feature in Lens and I miss it in Headlamp.

Improvement ideas

  • It would be helpful to have a filter for CRDs, as some you will never access
  • A way to exclude a single entry, if a plugin provides this feature
  • We could allow to disable this feature like the drawer one

@illume
Copy link
Contributor

illume commented May 27, 2025

Very cool.

A fix for this has been requested by a few people... One issue is here (with quite a few thumbs up from people):

The difference in the issue is, they are asking for a collapsible menu in the page.

@farodin91
Copy link
Contributor Author

The difference in the issue is, they are asking for a collapsible menu in the page.

I don't see a difference to the issue. Where do you see the difference?

@illume
Copy link
Contributor

illume commented May 27, 2025

@farodin91 I thought they were suggesting putting the table on the page, vs the sidebar?

@farodin91
Copy link
Contributor Author

Okay. In Openlens is it in sidebar.

@illume illume mentioned this pull request May 27, 2025
@ivelisseca
Copy link

Can I see a full screenshot of the table of contents (sidebar) and the page for scenario where there might be a lot of CRs? The small picture doesn't make it clear for me to see the scale of the impact.

@farodin91
Copy link
Contributor Author

image
I played around collapsing under customer resources think it is better.

@farodin91 farodin91 force-pushed the expand-crs-on-sidebar branch from 6989ded to d977cd8 Compare May 28, 2025 08:50
@sniok
Copy link
Contributor

sniok commented May 28, 2025

just a reminder that in the current sidebar implementation we also have tabs when sidebar is collapsed that don't support nesting

@farodin91
Copy link
Contributor Author

@sniok I will fix it for this view.

@farodin91 farodin91 force-pushed the expand-crs-on-sidebar branch 2 times, most recently from c1c16e5 to bb93978 Compare July 1, 2025 10:37
@farodin91 farodin91 marked this pull request as ready for review July 1, 2025 10:38
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 1, 2025
@k8s-ci-robot k8s-ci-robot requested a review from illume July 1, 2025 10:38
@farodin91
Copy link
Contributor Author

@sniok Multi layer navigation tabs are added.

@farodin91 farodin91 force-pushed the expand-crs-on-sidebar branch 2 times, most recently from 920adb1 to 94d84f1 Compare July 1, 2025 14:39
@farodin91 farodin91 force-pushed the expand-crs-on-sidebar branch from 94d84f1 to da76b79 Compare July 2, 2025 20:17
@farodin91
Copy link
Contributor Author

image

@farodin91 farodin91 force-pushed the expand-crs-on-sidebar branch from da76b79 to 4f6b727 Compare July 2, 2025 20:29
@farodin91
Copy link
Contributor Author

@illume would you like to review?

@farodin91
Copy link
Contributor Author

farodin91 commented Jul 3, 2025

I will extract two changes into extract PRs -> #3560 & #3561

@sniok
Copy link
Contributor

sniok commented Jul 7, 2025

multiple tab layers with horizontal scrolling is not a good UX, it was considered before but decided against it

@farodin91
Copy link
Contributor Author

@sniok Do you have a better idea?

Signed-off-by: farodin91 <github@jan-jansen.net>
@farodin91 farodin91 force-pushed the expand-crs-on-sidebar branch from 4d339c0 to 8e8a00b Compare July 7, 2025 20:48
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 7, 2025
@farodin91
Copy link
Contributor Author

Rebased and removed both included changes from #3560 and #3561.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Development

Successfully merging this pull request may close these issues.

6 participants