Skip to content

Improved usage of VDataTableVirtual#2291

Merged
grolu merged 9 commits intomasterfrom
enh/virtual-table-improvements
Mar 28, 2025
Merged

Improved usage of VDataTableVirtual#2291
grolu merged 9 commits intomasterfrom
enh/virtual-table-improvements

Conversation

@grolu
Copy link
Copy Markdown
Member

@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
@grolu
Improved usage of VDataVirtual
be66557 
- 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
@grolu
Merge branch 'master' into enh/virtual-table-improvements
3276b12
@gardener-robot gardener-robot added needs/review Needs review size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. 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
@ghost ghost 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
Copy Markdown

gardener-robot commented Feb 21, 2025

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

petersutter
petersutter reviewed Feb 26, 2025
View reviewed changes
Comment thread frontend/src/composables/useTwoTableLayout.js Outdated
const { height: containerHeight } = useElementSize(containerRef)

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

@petersutter petersutter Feb 26, 2025

Choose a reason for hiding this comment

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

why +1?

Copy link
Copy Markdown
Member Author

@grolu grolu Feb 26, 2025

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.

Comment thread frontend/src/composables/useTwoTableLayout.js Outdated
Comment thread frontend/src/composables/useTwoTableLayout.js
Comment thread frontend/src/views/GMembers.vue Outdated
Comment thread frontend/src/views/GSecrets.vue Outdated
Comment thread frontend/src/views/GSecrets.vue
Comment thread frontend/src/components/GShootListRow.vue Outdated
@petersutter
Copy link
Copy Markdown
Member

petersutter commented Feb 26, 2025

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
@ghost ghost 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
@grolu
Merge branch 'master' into enh/virtual-table-improvements
dc11e85 
# Conflicts:
#	frontend/src/views/GShootList.vue
@grolu
Copy link
Copy Markdown
Member Author

grolu commented Mar 19, 2025

select the worker colum -> uncaught error

Not related to this PR, closed with #2357

@gardener-robot-ci-1 gardener-robot-ci-1 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 Mar 19, 2025
@ghost ghost 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 Mar 19, 2025
petersutter
petersutter approved these changes Mar 28, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@petersutter petersutter left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/rebase Needs git rebase needs/review Needs review needs/second-opinion Needs second review by someone else labels Mar 28, 2025
@ghost ghost 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 Mar 28, 2025
@gardener-robot gardener-robot added the needs/second-opinion Needs second review by someone else label Mar 28, 2025
@petersutter petersutter 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 needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/second-opinion Needs second review by someone else labels Mar 28, 2025
@gardener-robot-ci-2 gardener-robot-ci-2 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 Mar 28, 2025
@grolu grolu merged commit 93d66a0 into master Mar 28, 2025
11 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Mar 28, 2025
@grolu grolu deleted the enh/virtual-table-improvements branch March 28, 2025 09:56
@grolu grolu added the area/ipcei IPCEI (Important Project of Common European Interest) label May 27, 2025
@grolu grolu mentioned this pull request May 27, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@petersutter petersutter petersutter approved these changes

@holgerkoser holgerkoser Awaiting requested review from holgerkoser holgerkoser is a code owner

Assignees

No one assigned

Labels

area/ipcei IPCEI (Important Project of Common European Interest) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. status/closed Issue is closed (either delivered or triaged)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@grolu @gardener-robot @petersutter @gardener-robot-ci-1 @gardener-robot-ci-2