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

Avoid loading anything else than core scripts/styles on empty/base renderAs tempalte #30472

Closed
wants to merge 1 commit into from

Conversation

juliusknorr
Copy link
Member

This PR will remove anything but the core js bundle from the base or empty template rendering. This is basically what is described in #10142

The existing RENDER_AS_BASE should be sufficient for this already and seems to be only used in Collabora/ONLYOFFICE these days anyways (as far as my git checkouts showed). We could even discuss if dropping the server.scss might be a good idea and instead just load the css variables.

Things kept:

  • Core js bundle
  • Server styles
  • CSS variables
  • Translations

Since this may be a breaking change, this should be added to the developer docs after merge.

@juliusknorr juliusknorr added enhancement 3. to review Waiting for reviews labels Jan 3, 2022
@juliusknorr juliusknorr requested review from skjnldsv, a team, ArtificialOwl, icewind1991, PVince81 and CarlSchwan and removed request for a team January 3, 2022 12:53
@skjnldsv
Copy link
Member

skjnldsv commented Jan 5, 2022

This will also be usefulfor the pdfviewer iirc. This is what we discussed last year, isn't it?
nextcloud/files_pdfviewer#202

@skjnldsv
Copy link
Member

skjnldsv commented Jan 5, 2022

Also, does it make sense to introduce a new render constant like RENDER_AS_CLEAN ?

@juliusknorr
Copy link
Member Author

I don't see a reason to introduce yet another one if we manage to clean up what is being loaded in RENDER_AS_BASE. Though if you see any use case of having two separate ones, we could add that of course.

@skjnldsv
Copy link
Member

Nope, all good for me @juliushaertl 🚀

@blizzz blizzz added this to the Nextcloud 25 milestone Apr 13, 2022
@PVince81
Copy link
Member

revive for 25 ?

This was referenced Aug 12, 2022
This was referenced Aug 24, 2022
This was referenced Sep 6, 2022
@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@juliusknorr
Copy link
Member Author

After having another look I'll double check what the existing ones in RENDER_AS_BASE actually do and also try to document this for the other types based on the current behaviour.

@blizzz blizzz mentioned this pull request Feb 1, 2023
This was referenced May 3, 2023
@blizzz blizzz mentioned this pull request May 17, 2023
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
@blizzz blizzz mentioned this pull request Nov 14, 2023
@juliusknorr
Copy link
Member Author

We switched to a different approach for Collabora now which does not require us to do another template rendering, so I'll close this due to lack of time to finish. Anyone is welcome to pick this up of course if needed.

@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Feb 23, 2024
@skjnldsv skjnldsv deleted the enh/base-template branch March 14, 2024 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants