Skip to content

[FIX] Loading theme CSS on first server startup#13953

Merged
sampaiodiego merged 1 commit intodevelopfrom
fix-first-theme-css-load
Apr 2, 2019
Merged

[FIX] Loading theme CSS on first server startup#13953
sampaiodiego merged 1 commit intodevelopfrom
fix-first-theme-css-load

Conversation

@sampaiodiego
Copy link
Copy Markdown
Member

Change the technique used to serve the theme.css file.

Previous code was overwriting an internal meteor function that read/validate all assets of the app and if perhaps the CSS theme was already "ready" (the less was compiled), it was then added to assets list. This was causing an issue though, that on the first start of a server, where the compiled CSS was not yet saved on DB, then the theme.css was being served. Causing the UI to look like this:

image

Previously, to fix the above issue a restart was required.

@sampaiodiego sampaiodiego added this to the 1.0.0 milestone Mar 29, 2019
@sampaiodiego sampaiodiego changed the title [IMPROVE] Inject tag to load theme.css [FIX] Loading theme CSS on first load Apr 1, 2019
@sampaiodiego sampaiodiego changed the title [FIX] Loading theme CSS on first load [FIX] Loading theme CSS on first server startup Apr 1, 2019
res.setHeader('Content-Type', 'text/css; charset=UTF-8');
res.setHeader('Content-Length', currentSize);
res.setHeader('ETag', `"${ currentHash }"`);
res.write(theme.getCss());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would this actually exist?

I know let is defined outside this scope... but will this actually persist?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes =)

Variables declared by let have their scope in the block for which they are defined, as well as in any contained sub-blocks.

source

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

actually, if you're worried about theme variable, it is a const that was being used before as well

@sampaiodiego sampaiodiego merged commit 42b57f9 into develop Apr 2, 2019
@sampaiodiego sampaiodiego deleted the fix-first-theme-css-load branch April 2, 2019 00:52
@facundomedica
Copy link
Copy Markdown

@sampaiodiego could this be the reason that breaks the loading of theme.css when using Rocketchat in a subdirectory? I'm testing on develop branch and found out that now it asks for /theme.css which doesn't exist.

@sampaiodiego
Copy link
Copy Markdown
Member Author

@sampaiodiego could this be the reason that breaks the loading of theme.css when using Rocketchat in a subdirectory? I'm testing on develop branch and found out that now it asks for /theme.css which doesn't exist.

I can take a look..

@facundomedica
Copy link
Copy Markdown

facundomedica commented Apr 5, 2019

Thanks for the quick response! More info: It also happens in 1.0.0-rc.0 but not in 1.0.0-beta.2

@sampaiodiego
Copy link
Copy Markdown
Member Author

@facundomedica can you please help testing #14015 ?

@facundomedica
Copy link
Copy Markdown

@sampaiodiego thank you, is there a way to have this uploaded to docker hub? I've been testing from there and I don't have the set up to run it in other way

@sampaiodiego
Copy link
Copy Markdown
Member Author

@facundomedica you can test with docker image rocketchat/rocket.chat:pr-14015 .. from my tests it is now ok, so went ahead and merged the PR.

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.

5 participants