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

perf(dashboard): Stop loading the viewer on the dashboard #43407

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Feb 6, 2024

Summary

Viewer is ~4MB of JS assets for loading, this is not needed on the dashboard and the sidebar is also not used on the dashboard - there is no sidebar at all.

before after
Screenshot_20240206_220934 Screenshot_20240206_221022
Screenshot_20240206_221121 Screenshot_20240206_221059

Checklist

Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

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

BTW, should it be considered a breaking change?

For example, if some community app uses dashboard api v1 and Viewer

@susnux susnux force-pushed the fix/dont-load-viewer-on-dashboard branch from b487ee8 to 3b6496b Compare February 7, 2024 00:42
@susnux
Copy link
Contributor Author

susnux commented Feb 7, 2024

@AndyScherzinger when emulating "fast 3G"-network connection on chrome the improvements are quite nice:

If you start on the login page with an empty cache the loading time for the first page (dashboard) is reduced by 44%:

before after
Screenshot_20240207_013943 Screenshot_20240207_014132

@nickvergessen
Copy link
Member

BTW, should it be considered a breaking change?
For example, if some community app uses dashboard api v1 and Viewer

I would say mentioning it in the developer docs doesn't hurt. The fix is also easy (emit the event yourself)

@skjnldsv
Copy link
Member

skjnldsv commented Feb 7, 2024

I would say mentioning it in the developer docs doesn't hurt. The fix is also easy (emit the event yourself)

Which is also not really a fix, they should have done that since the beginning.
never rely on others triggering events for you :)

@susnux
Copy link
Contributor Author

susnux commented Feb 7, 2024

Documentation: nextcloud/documentation#11513

@st3iny st3iny merged commit 0171812 into master Feb 7, 2024
141 checks passed
@st3iny st3iny deleted the fix/dont-load-viewer-on-dashboard branch February 7, 2024 13:37
@blizzz blizzz mentioned this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants