Skip to content
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

Improved usage of VDataTableVirtual #2291

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

grolu
Copy link
Contributor

@grolu grolu commented Feb 4, 2025

  • Pass ref to ensure correct row height measurement
  • Introduced GScrollContainer Component to indicate more content is available
  • Optimized worker, ticket labels and access restrictions layout on cluster list
  • Use VDataVirtual for Secret & Member Page
  • Introduced useTwoTableLayout composable for optimized two table page layouting

What this PR does / why we need it:

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Release note:


grolu added 2 commits February 4, 2025 15:15
- Pass ref to ensure correct row height measurement
- Introduced GScrollContainer Component to indicate more content is available
- Optimized worker, ticket labels and access restrictions layout on cluster list
- Use VDataVirtual for Secret & Member Page
- Introduced useTwoTableLayout composable for optimized two table page layouting
@gardener-robot gardener-robot added needs/review Needs review size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py) needs/second-opinion Needs second review by someone else labels Feb 4, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 4, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 4, 2025
@grolu grolu changed the title [DRAFT] Improved usage of VDataVirtual [DRAFT] Improved usage of VDataTableVirtual Feb 5, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 6, 2025
@grolu grolu marked this pull request as ready for review February 7, 2025 12:00
@grolu grolu changed the title [DRAFT] Improved usage of VDataTableVirtual Improved usage of VDataTableVirtual Feb 11, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 21, 2025
@gardener-robot-ci-1 gardener-robot-ci-1 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 21, 2025
@gardener-robot gardener-robot added the needs/rebase Needs git rebase label Feb 21, 2025
@gardener-robot
Copy link

@grolu You need rebase this pull request with latest master branch. Please check.

const { height: containerHeight } = useElementSize(containerRef)

const desiredFirstTableHeight = computed(() => {
const contentHeight = firstTableItemCount.value * (itemHeight + 1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why +1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I added this to try to compensate the fact that the table adds srollbars even if the calculated height matches exactly the content height. Looking at it now it seems to only make it worse. Let's remove it.

@petersutter
Copy link
Member

there is also a conflict that has to be solved. In addition, I noticed the following bug (as discussed offline)

Go to some Project, make sure to deselect the worker column from the table options
Navigate to ALL PROJECTS
select the worker colum -> uncaught error
chunk-UDQV3BQT.js?v=71e4a3da:2332 Uncaught TypeError: Cannot set properties of undefined (setting 'workers')
at Proxy.setSelectedHeader (GShootList.vue:766:40)
at Proxy.onSetSelectedHeader (GTableColumnSelection.vue:159:3)
at onUpdate:modelValue (GTableColumnSelection.vue:58:32)

@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Feb 26, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 26, 2025
@gardener-robot-ci-3 gardener-robot-ci-3 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/rebase Needs git rebase needs/review Needs review needs/second-opinion Needs second review by someone else size/xl Size of pull request is huge (see gardener-robot robot/bots/size.py)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants