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

Performance: Don't allow column definiton changes in sortable table #6389

Closed
wants to merge 2 commits into from

Conversation

nwmac
Copy link
Member

@nwmac nwmac commented Jul 18, 2022

This PR makes a small improvement to the sortable table.

On my large cluster, schema updates occur quite frequently, which causes the headers computed to re-evaluate.

This PR watches the headers and only updates a new tableHeaders property when the actual set of columns changes (checks names). This means is a column definition changes while viewing a table, this will be not be reflected - but we do not expect this to happen and currently this does not happen.

@nwmac nwmac added this to the v2.6.7 milestone Jul 18, 2022
@nwmac nwmac self-assigned this Jul 18, 2022
@nwmac nwmac marked this pull request as ready for review July 19, 2022 07:40
@richard-cox richard-cox self-requested a review July 19, 2022 15:48
Copy link
Member

@richard-cox richard-cox left a comment

Choose a reason for hiding this comment

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

  • The reference to headers here should be the new tableHeaders
  • This reference to headers as well should be tableHeaders

@@ -52,14 +52,15 @@ export default {
},

data() {
const headers = this.tableHeaders || [];
Copy link
Member

Choose a reason for hiding this comment

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

These references should stay as header. This fn is called once when the component is created and tableHeaders hasn't been initialised yet (so there's no sort order set, for instance on the workloads list)

@nwmac nwmac marked this pull request as draft July 20, 2022 16:34
@nwmac nwmac removed this from the v2.6.7 milestone Jul 22, 2022
@nwmac
Copy link
Member Author

nwmac commented Jul 25, 2022

Closed in favor of web worker PR

@nwmac nwmac closed this Jul 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants