codingdavinci / relaunch2018

This is the new Coding da Vinci website (online since September 2020).
https://codingdavinci.de
GNU General Public License v2.0
1 stars 1 forks source link

CSS-Dateien aggregieren should be possible #196

Closed mbuechner closed 3 years ago

mbuechner commented 4 years ago

To enhance the performance of the website the option 'CSS-Dateien aggregieren' under 'Leistung' should be enabled. If I do at the moment Drupal can't do the job correctly and the website is displayed without any CSS styling. This should be fixed!

https://www.drupal.org/project/minifyjs https://www.drupal.org/project/minifyhtml

Both module are totally worth to install as well!

Snater commented 4 years ago

At some point there was some problem with aggregating styles, yet the checkboxes on "Leistung" are ticked and CSS works correctly at the moment as far as I can see.

I don't see a real need for the modules. I'd rather opt for not having these as additional dependencies. We can try to enable BigPipe as per #197 which might have a more serious performance impact.

mbuechner commented 3 years ago

Doesn't work on test.condingdavinci.de. kk

Snater commented 3 years ago

I cannot access the test deployment yet. Is there maybe a settings.local.php set up that would overwrite the aggregation settings? Locally, I have set up a settings.local.php that disables aggregation (by setting $config['system.performance']['css']['preprocess'] = FALSE; and $config['system.performance']['js']['preprocess'] = FALSE;, but when I remove it, files get aggregated just like they had been on the staging deployment.

mbuechner commented 3 years ago

We're using the Docker container build from https://github.com/codingdavinci/relaunch2018

If we need a settings.local.php (with any configuration) simply generate one in this repro, a new Docker container will be build and the we can deploy it to test.codingdavinci.de afterwards.

Snater commented 3 years ago

There is no need for a settings.local.php. The question was rather if you might be setting up one separately for the deployment? But I guess you are not. Anyway, I just enabled the aggregation options on the test deployment and things seem to work?

lucyWMDE commented 3 years ago

Not sure what this change might have affected... but we do now have a few problems

I've had a cursory look around the site and can't find anything else that's a problem. But perhaps @mbuechner has suggestions of where else to do more detailed testing?

mbuechner commented 3 years ago

Anyway, I just enabled the aggregation options on the test deployment and things seem to work?

Nope, still doesn't work! Don't forget to clear cache after enabling/disabling aggregation.

mbuechner commented 3 years ago

Aggregation of JS files does work neither.

If you enable the menu at https://test.codingdavinci.de/de/projekte doesn't work. 😢

lucyWMDE commented 3 years ago

I don't know what you did @mbuechner but the menu hamburger is back on dataset and project overview pages now, at least for me (on firefox). Filter link styles still need fixing.

Snater commented 3 years ago

I fail to reproduce this. Enabling aggregation, clearing all Drupal caches, hard-reloading the browser, things work. Chrome. Firefox. Maybe I am fundamentally misunderstanding this issue?

lucyWMDE commented 3 years ago

in the filter (left sidebar) there should be many tag link styles, as seen on http://codingdavinci.snater.com/de/daten image

Instead all tag link styles are overridden, as seen on https://test.codingdavinci.de/de/daten image

This is true for both the dataset overview page and the project overview page. As I said - I'm not sure if this bug is related to CSS aggregation... if it should have it's own ticket please create one.

The menu hamburger issue no longer seems to be a problem. I can't comment on any other issues. Maybe @mbuechner has something to add?

lucyWMDE commented 3 years ago

sorry - I'm not sure where this filter tag link styles problem belongs... I've also referred to it in issue #242. I don't know of any further CSS aggregation issues and leave it to @mbuechner to comment or close this issue if everything is resolved.

Snater commented 3 years ago

The filter styles were indeed an effect of #242, but were fixed per https://github.com/codingdavinci/relaunch2018/commit/95c81707e1baa4e84f24fa96c30a19375677162d yet either the deployment took a while or things were just cached. Anyway, it seems to be correct on the test deployment now.

As for the aggregation of styles and JS: cdv_css cdv_js This is as expected, except for that one 404 which I have not experienced anytime before. Might be related to some config change? Anyway it would be a new issue.

I'm closing this again.