Skip to content

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 15, 2023

Fix regression from #23481.

The conditional on the CSS import was being stripped away by webpack's css-loader, resulting in the dark theme always loading. The old syntax with @import nested inside @media also did not work as css-loader (rightfully) ignores such non-standard @import syntax that was previously supported by Less.

Unfortunately, we have to re-introduce postcss to the CSS pipeline to fix this and I loaded only the minimal plugins to make it work.

There is one variant of the fix that does work without postcss, which is to exclude the file from transpilation but I did not consider it as it would have meant the @import was being done without a version suffix in the URL, which would have caused cache issue.

Related: webpack-contrib/css-loader#1503

Fix regression from go-gitea#23481.

The conditional CSS import was being stripped away by webpack's
`css-loader`, resulting in the dark theme always loading. Unfortunately,
we have to re-introduce postcss to the CSS pipeline to fix this and I
loaded only the minimal plugins to make it work.

Related: webpack-contrib/css-loader#1503
@silverwind silverwind added this to the 1.20.0 milestone Mar 15, 2023
@silverwind silverwind added the topic/ui Change the appearance of the Gitea UI label Mar 15, 2023
Copy link
Member

@delvh delvh left a comment

Choose a reason for hiding this comment

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

We should probably add a comment on the specific import line to detail that this (only) works because postcss converts the conditional import into inline code.

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 15, 2023
@silverwind
Copy link
Member Author

Agree, added the comment.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 15, 2023
@delvh delvh added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
@techknowlogick techknowlogick merged commit 19cbd5c into go-gitea:main Mar 15, 2023
@silverwind silverwind deleted the fix-theme-auto branch March 15, 2023 21:16
@jolheiser jolheiser removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Mar 15, 2023
@silverwind
Copy link
Member Author

silverwind commented Mar 15, 2023

Looks we've hit an actual webpack bug, watch webpack-contrib/css-loader#1503 for progress. Once fixed, we should be able to remove postcss again.

silverwind added a commit to silverwind/gitea that referenced this pull request Mar 16, 2023
Fix regression from go-gitea#23481.

The conditional on the CSS import was being stripped away by webpack's
`css-loader`, resulting in the dark theme always loading. The old syntax
with `@import` nested inside `@media` also did not work as `css-loader`
(rightfully) ignores such non-standard `@import` syntax that was
previously supported by Less.

Unfortunately, we have to re-introduce postcss to the CSS pipeline to
fix this and I loaded only the minimal plugins to make it work.

There is one variant of the fix that does work without postcss, which is
to exclude the file from transpilation but I did not consider it as it
would have meant the `@import` was being done without a version suffix
in the URL, which would have caused cache issue.

Related: webpack-contrib/css-loader#1503

---------

Co-authored-by: John Olheiser <john.olheiser@gmail.com>
zjjhot added a commit to zjjhot/gitea that referenced this pull request Mar 16, 2023
* giteaofficial/main:
  [skip ci] Updated translations via Crowdin
  Replace `repo.namedBlob` by `git.TreeEntry`. (go-gitea#22898)
  Fix theme-auto loading (go-gitea#23504)
  Update path to docs theme file (go-gitea#23502)
  Use arm image for arm runner (go-gitea#23503)
techknowlogick added a commit that referenced this pull request Mar 16, 2023
Follow-up and proper fix for
#23504

Update to
[mini-css-extract-plugin@2.7.4](https://github.com/webpack-contrib/mini-css-extract-plugin/releases/tag/v2.7.4)
which fixes our specific issue described in
webpack-contrib/css-loader#1503 and which
allows us to again drop the postcss dependency.

Backport of this is not necessary as I have included it in
#23508.

Co-authored-by: techknowlogick <techknowlogick@gitea.io>
techknowlogick pushed a commit that referenced this pull request Mar 17, 2023
Backport #23481,
#23504 and
#23520 to 1.19, just so we have an
easier time with future backports.

Seems to work on a basic level. There was a merge conflict in
`RepoActionView.vue`, otherwise it merged cleanly.

---------

Co-authored-by: John Olheiser <john.olheiser@gmail.com>
Co-authored-by: Lauris BH <lauris@nix.lv>
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants