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

View subscription refactor and bug fixes #1021

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

AaronPlave
Copy link
Contributor

@AaronPlave AaronPlave commented Nov 27, 2023

Partially addresses #1012 by resolving the underlying performance issue with the view subscription. Previously the view subscription included view definitions but this PR removes definitions from the subscription and instead fetches the full view on demand. This PR also fixes:

  • A small UI issue related to context menu escaping closing the open view modal
  • Issue deleting views as an admin where fix was to not rely on the userViews list since the admin might not be the owner of the view but could still have delete permissions (which will be checked downstream anyway).

@AaronPlave AaronPlave marked this pull request as ready for review November 27, 2023 23:59
@AaronPlave AaronPlave requested a review from a team as a code owner November 27, 2023 23:59
@AaronPlave AaronPlave force-pushed the feat/view-loading-perf-and-fixes branch from 4ed879e to 0a2af0f Compare November 28, 2023 00:07
@AaronPlave AaronPlave force-pushed the feat/view-loading-perf-and-fixes branch from 0a2af0f to 0e35a23 Compare December 1, 2023 00:40
@AaronPlave AaronPlave force-pushed the feat/view-loading-perf-and-fixes branch from 0e35a23 to ac81c1d Compare December 1, 2023 00:48
@AaronPlave AaronPlave force-pushed the feat/view-loading-perf-and-fixes branch from ac81c1d to 2aa76b3 Compare December 6, 2023 16:30
src/utilities/effects.ts Outdated Show resolved Hide resolved
@AaronPlave AaronPlave requested a review from duranb December 8, 2023 16:24
@AaronPlave AaronPlave force-pushed the feat/view-loading-perf-and-fixes branch from c1d4848 to 7a9a9f6 Compare December 8, 2023 16:24
Copy link
Collaborator

@duranb duranb left a comment

Choose a reason for hiding this comment

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

looks good!

@AaronPlave AaronPlave force-pushed the feat/view-loading-perf-and-fixes branch from 7a9a9f6 to b9c758d Compare December 11, 2023 16:49
@AaronPlave AaronPlave merged commit f39f30d into develop Dec 11, 2023
4 checks passed
@AaronPlave AaronPlave deleted the feat/view-loading-perf-and-fixes branch December 11, 2023 19:36
@AaronPlave AaronPlave self-assigned this Dec 11, 2023
@AaronPlave AaronPlave added the performance A code change that improves performance label Dec 11, 2023
JosephVolosin pushed a commit that referenced this pull request Aug 20, 2024
* Use capture for context menu keydown and stop propagation on escape keydown if shown
* Do not include view definition in view subscription and full view on demand. 
* Fix bug with deletion of views.
* Fix delete view permissions check
JosephVolosin pushed a commit that referenced this pull request Oct 21, 2024
* Use capture for context menu keydown and stop propagation on escape keydown if shown
* Do not include view definition in view subscription and full view on demand. 
* Fix bug with deletion of views.
* Fix delete view permissions check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance A code change that improves performance
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants