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

Replace Less #15565

Closed
silverwind opened this issue Apr 21, 2021 · 3 comments · Fixed by #23481
Closed

Replace Less #15565

silverwind opened this issue Apr 21, 2021 · 3 comments · Fixed by #23481
Labels
topic/ui Change the appearance of the Gitea UI type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@silverwind
Copy link
Member

silverwind commented Apr 21, 2021

Less is a rather dead project which does not support recent CSS features as seen in #15520. I suggest moving to either SCSS or plain CSS.

Thoughts on SCSS:

  • Supports pretty much everything and more that Less does
  • Is 4 times more populare then Less
  • Might be slower to compile with their Dart implementation
  • Like less, it encourages the bad practice of deeply nesting selectors, resulting in unnecessarily big CSS output

Thoughts on CSS:

  • Instant build, no compilation required
  • Obviously supports all CSS features out of the box
  • Does not support nesting which can be both a blessing (lightweight output) and a curse (impractical sometimes)
  • Best tooling support
@silverwind silverwind added type/proposal The new feature has not been accepted yet but needs to be discussed first. topic/ui Change the appearance of the Gitea UI labels Apr 21, 2021
@silverwind
Copy link
Member Author

silverwind commented Oct 22, 2022

I think plain CSS with https://www.npmjs.com/package/postcss-nesting is the way to go nowadays. Once enough browsers support native CSS nesting, the plugin can just be removed. This is btw also what vite recommends.

@silverwind
Copy link
Member Author

silverwind commented Dec 4, 2022

I wonder whether we should just convert the Less to CSS and commit the output as-is, without any nesting.

It will produce long selectors for the nesting cases, but personally I prefer no nesting because it'll make the selectors from browser devtools CTRL-F able in the source which is very helpful when searching for specific selectors.

Also, not using nesting encourages developers to write shorter and better selectors. I think it's a net benefit to not have nesting.

@delvh
Copy link
Member

delvh commented Dec 4, 2022

Absolutely agreed.
It has another huge benefit: You can tell immediately what style classes will interfere with what you are doing.
At the moment, I simply cannot do anything untested in the frontend, simply because there might be a four times nested selector I of course didn't think about that destroys what I'm trying to do.
Let's use the following hypothetical example:
I add a link somewhere.
Unfortunately, that link is inside a div that is inside an element with the ui class, and hence there is a selector that hides the link. Yay. Who would've guessed that my link isn't shown at all?

silverwind added a commit to silverwind/gitea that referenced this issue Mar 15, 2023
techknowlogick pushed a commit that referenced this issue Mar 15, 2023
Ran most of the Less files through the Less compiler and Prettier and
then followed up with a round of manual fixes.

The Less compiler had unfortunately stripped all `//` style comments
that I had to restore (It did preserve `/* */` comments). Other fixes
include duplicate selector removal which were revealed after the
transpilation and which weren't caught by stylelint before but now are.

Fixes: #15565
silverwind added a commit to silverwind/gitea that referenced this issue Mar 16, 2023
Ran most of the Less files through the Less compiler and Prettier and
then followed up with a round of manual fixes.

The Less compiler had unfortunately stripped all `//` style comments
that I had to restore (It did preserve `/* */` comments). Other fixes
include duplicate selector removal which were revealed after the
transpilation and which weren't caught by stylelint before but now are.

Fixes: go-gitea#15565
@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
topic/ui Change the appearance of the Gitea UI type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants