[Thread] CSS Optimization and Testing Summary

[kdumontnu]

I wanted to put together a list of proposals for CSS refactor and consolidation.

I've been working on some styling bugfixes lately and found the state of the CSS code to be quite frustrating. I wanted to discuss some options to improve performance, consistency, and reduce bugs.

Problems

UI Bugs

I'm fairly new to this project, but I've found quite a few UI bugs that really take away from the professional feel of Gitea. Many are reported in open issues, but most are not.

Here are a couple I found just now clicking around:

Consistency

There is a lot of inconsistency with how classes are defined and styled. There is some use of semantic.css, but also helpers.less (which is completely counter to Semantic UI ideology), and extreme usage of class overrides in web_src/less.
This makes it very difficult to develop, but also results in some visual discrepancies. For instance, see the badges being styled 3 different ways below:

Further, we are effectively hard-forked from Fomantic source because of changes in ./web_src/fomantic/build/semantic.css. I'm sure there were good reasons for these changes, but it will increasingly become impossible to update the dependency (btw, Semantic has support for theming, even demonstrating how to emulate GitHub's old styling).

Performance

index.css is flagged by Chrome dev tools as the highest network utilization when loading a page and a critical-path file, which takes 500ms + to load on my computer with a good internet connection (with caching disabled).

The is largely because of the general file size.
Priority: Highest
Encoded Data: 128 kB
Decoded Body: 845 kB

The utilization of CSS classes on a given page is extremely low. 96% of the css classes are unused on any given page:

Proposed Solutions

I'm not writing this for the purpose of putting the project or anyone down - there is incredible progress on the app! However, a lot of the focus is on adding new features without much consideration for the architecture. It's less glorious, but important stuff 😄

I propose the following tentative solutions:

• Create a style guide for Gitea
• Review new PRs against the style guide
• Add a CI test for CSS - there are many options to choose from (BackstopJS, Karma, etc.)
• Use webpack plugin to remove unused CSS (PurifyCSS)
• Refactor to remove ALL files in web_src/less & use Semantic's supported .variables and .overrides, beginning with helpers.less and all _*.less files
• Revert all changes to web_src/fomantic/build/semantic.css

Questions

Am I missing anything else?
Is there significant desire to continue using Fomantic UI? It would be very painful to move away, but something like Tailwind would solve half of the suggestions above (including the precompiler)

This is certainly a huge undertaking, but I wanted to test to see if others agree with this analysis or it's not of interest. I also don't want to bring up all of these issues without helping towards a solution, so I plan to contribute to some of these resolutions.

[techknowlogick]

Without getting into all the points above, technically we are using fomantic which is a fork of semantic for security & accessibility reasons.

[lunny]

@kdumontnu Thanks for your proposals and ideas about community contribution styles. I think we can start with some easier one, i.e. introducing PurifyCSS

[CL-Jeremy]

Sorry for not noticing this. I dropped a comment in your merged PR (traced from Git blame) and I apologize for that.

The Gitea project originated from Gogs, which is maintained more or less by a single person. As such the workflow with styling has not been particularly thoughtful, with class-based overriding being commonplace from the beginning. The fact that stale code went dead unnoticed and made through multiple releases is unfortunate, and suggests that this type of thread is exactly needed.

Regarding semantic.css, from what I can tell it's just normal usage with several custom parameters set in site variables. I don't consider that as a problem with how Semantic/Fomantic UI is used.

helpers.less started only recently as a result of having too many class names. Think of it as disintegrating semantic classes (which could have long names) into rank-marked features, which should be limited in usage (I apologize for not using them exactly responsibly, good those are not merged yet). I agree it's no good convention, that architectural changes to styling would become inevitable, but it did solve the problem @silverwind had, and to some extent pretty nicely.

The example of label padding is exactly something hard to customize with Semantic UI, and in their demo they were overriding them as well, but the classes are separated by element instead of by context. Factoring out those basic redefinitions from _base.less etc. is of course a means to help with the current situation. In fact it seems a redesign of color classes has been planned already, so that would also help.

[kdumontnu]

Sorry for not noticing this. I dropped a comment in your merged PR (traced from Git blame) and I apologize for that.

No need to apologize - it was helpful to point out and a great example of the difficulty with styling (PR for reference).

Thanks for the background! Super helpful to understand.

I would really like this to evolve into a more detailed roadmap for improving the CSS architecture, so it would be helpful to catalog ongoing efforts and settle on a trajectory. I attempted to put the suggested high-level items in order of priority. I really think having a style-guide that explains the Gitea style ideology/methodology is a necessary beacon for contributors to build towards.

It's interesting that helpers.less seems to follow the Tailwind ideology that broad css classes are futile and you will always riddle them with overrides, which is the opposite of fomantic/semantic. In my opinion, we need to pick a direction, document it, and stick with it as best possible. Which leads me to my two main questions:

Do we like Fomantic/Semantic generalized-styles or do we want to move to a "utility-first" style?
It wouldn't be very difficult to start porting the app over to something like Tailwind (and support both in the interim), BUT unless there is 110% support it risks being aborted and adding yet another hybrid section to the styling.

Is there any preference for Karma vs BackstopJS or any other UI regression testing?
Having a CI utility to test rendered output for changes will have the biggest impact and imo a prerequisite to any larger rearchitectures. I've used Karma in the past, but backstop seems quite promising.

[CL-Jeremy]

One other thing with Semantic/Fomantic UI is the extensive use of em-values of fractions of 7, in order to recover 10-based pixel values (e.g. default line height: 14 px * 1.4285 em = 19.999 px), which has created noticeable glitches (cf. #14704 (comment)) on high DPI devices or when zoomed in on desktop (I suppose pinch-zoom wouldn't count here). I imagine this could be solved using calc() if we employ some sort of post-processing step in a plug-in.

[CL-Jeremy]

In CL-Jeremy@090cfa3 and CL-Jeremy@6403812#diff-5d1373f9290f762a4751c497b0e8be279eb04caa3a7cdc2fd3f42b9f505a83be, a general override for vanilla-sized rectangular labels has been added to match their original styling in Gogs. Since these will not be merged until v1.15.0, it might be nice to port these changes to master earlier.

[silverwind]

Do we like Fomantic/Semantic generalized-styles or do we want to move to a "utility-first" style?

I think we want both. Single-purpose helper classes for things like margin, padding etc. (to fine-tune size and positions) and generalized classes for buttons, inputs and more involved styling.

The reason the CSS is 800kB is because the CSS that fomantic generates is insanely bloated output from their Less code (.ui.ui.ui.ui.table for example and a ton of almost-duplication in the file).

We need to get off fomantic, ideally to a complete framework-less solution that we can tailor to our needs so that the end result is minimal CSS (I imagine 50kB ought to be enough).

We can gradually migrate off Fomantic by step-by-step replacing the individual modules which will lighten the CSS/JS build gradually as well. Some components will be harder to replace, especially the ones that use JS like the Dropdown.

Is there any preference for Karma vs BackstopJS or any other UI regression testing?

I see some use in such tools but I never used them. Screenshot diffing on headless chrome does sound like the way to go and I think it may actually be nice to have screenshot diffs when reviewing PRs, but downside is the screenshots need to be checked in with the code.

[silverwind]

Also I think we should move to plain CSS over Less. No preprocessor means faster compilation and we barely use any Less features besides nesting (which I think is a bad practice in itself as it somewhat encourages nesting which leads to overly long CSS selectors).

[kdumontnu]

it may actually be nice to have screenshot diffs when reviewing PRs, but downside is the screenshots need to be checked in with the code.

Agreed. It would be really nice to set this up. I did some local testing with BackstopJS and it worked really well.

Committing the screenshots with the code directly in the git repo might become unbearable, but it seems like this would be the perfect case for git LFS. At first I thought that might add too much complexity (as a dev dependency), but I think if we go the LFS route and people clone the repo without LFS installed git just won't resolve the screenshot files (no error). So, we can add a note in the docs saying "if you want to run UI tests locally you'll need to install git LFS" otherwise we just handle it in CI.

Unless that rings any alarm bells I can try to work on a PR.

[silverwind]

AFAIK Github does not support LFS and I think the big benefit would be to have visual diffs in the PRs so I guess the files would have to live in the repo for GitHub to see them. Personally, I've strayed from using LFS so far and have not activated it on my Gitea instances because I prefer to not require git extensions on clone.

[kdumontnu]

AFAIK Github does not support LFS

How so? I use git LFS with GitHub all the time. Here's a demo I put together

LFS would only be required as a dev dependency for someone making changes that effect the UI, not to build or any backend changes. If you clone without LFS installed you just get pointer files for the screenshots. Alternatively we could let CI handle generating and committing the images on PRs, but then you will have to pull your branch any time you push on a PR with a UI change...

Unfortunately, I don't think this solution would be feasible without LFS (the repo would grow too large). I guess that's why tools like Cypress exist to manage it externally from the git repo.

Edit: I did realize you get a quota with LFS, however. It's not too expensive, but it is one more think to keep in mind.

[silverwind]

TIL. Seems free accounts do get 2 GB LFS quota which is probably plenty for a few screenshots. There seems to be some transfer quota thought and if I read this correctly, it means any clone would add up to this quota, meaning anonymous users could push the repo to the free limit.

[silverwind]

I guess a few size-optimized PNGs would be okay to have in the repo and it will save us from future headaches with LFS quotas. It would probably boil down to around 40 PNGs to cover the most important pages in light and dark theme. One such example.

[kdumontnu]

A full page screenshot (note, you can also capture single elements or regions) at 768x1024 is about 65KB. Doing some back-envelope math, if ~ 1 in 5 PRs changes the UI, we'll have about 1 updated image per day (using the past week as an example). So 65KB * 30 days = ~2MB/month added to repo size. That's not so bad...

We just can't test 4K resolution 😅
and we'd need to separate elements that could change across multiple screenshots - for example, we should capture the navbar in one screen and omit it from every other screen.

[kdumontnu]

Netlify is going through a very similar process: https://www.netlify.com/blog/2021/03/23/from-semantic-css-to-tailwind-refactoring-the-netlify-ui-codebase/

They are using Percy storybook for regression testing.

[lunny]

How about bulma ?

[silverwind]

I would not consider bulma until they support css vars (stale PR jgthms/bulma#2981).

I think generally we want to combine both helper-style classes (maybe tailwind, but I'm not convinced) and more general classes for often re-used classes like buttons. That way we are not dependant on any frameworks which may become bloated/outdated/unmaintained.

[silverwind]

As for unit-testing frontend code, I think we could use jest. Any objections?

My only one would be that it needs transpilation for ESM support, jestjs/jest#9430, but it's not a blocker imho.

[kdumontnu]

As for unit-testing frontend code, I think we could use jest. Any objections?

I'm good with Jest, but it only tests javascript code, right? We would need something else for CSS/UI?

[silverwind]

Yes, for actual UI interaction tests, I would probably have a look at https://testing-library.com/ but given that such tools expect the DOM to be rendered by JS, it might not be a good fit after all. I guess it will be kind of hard to integrate the backend rendering into frontend tests. I guess something based on a headless browser would still be needed for our case.

[kdumontnu]

Yes, for actual UI interaction tests, I would probably have a look at https://testing-library.com/ but given that such tools expect the DOM to be rendered by JS, it might not be a good fit after all. I guess it will be kind of hard to integrate the backend rendering into frontend tests. I guess something based on a headless browser would still be needed for our case.

testing-library looks neat and it's one step in the right direction, but imo the biggest problem it doesn't solve is when a developer changes a CSS property or changes an HTML class and breaks another part of the app by unknowingly matching a selector. It would also take a lot of effort to test different screen sizes, etc as opposed to a screenshot based test.

[silverwind]

Yes, of course CSS regression can only be tested with screenshots, so:

• for unit tests, use jest
• for interaction tests, use something like playwright/puppeteer and maybe testing-library
• for css tests, use screenshots via playwright/puppeteer

[silverwind]

https://github.com/testing-library/pptr-testing-library looks like something that could work. You could probably also produce and compare screenshots during these test runs. Could wrap those tests into jest test cases.

[silverwind]

Initial jest integration in #15315.

[kdumontnu]

Closing this in favor of #18302