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

Merge SCSS in a single components.scss file #2608

Conversation

ClearlyClaire
Copy link

Doing this pains me, but one of the worst part of porting upstream changes is dealing with components.scss changes, as the contents are scattered across multiple files.

@ClearlyClaire
Copy link
Author

I thought this would be relatively simple, but class definitions really have moved around a bunch… this is going to be a pain either way… and mastodon#28119 has 1200 line changes in components.scss 😩

@ClearlyClaire ClearlyClaire force-pushed the glitch-soc/refactor/monolithic-components-css branch 9 times, most recently from de5cc30 to b6b5aa9 Compare February 7, 2024 22:43
@ClearlyClaire
Copy link
Author

I again spent many hours on this, and there are still many more hours to spend. I'm still not sure this is a good solution, considering the whole “monolithic SCSS” mess is awful, but the upshots are:

  • it helps identifying actual differences between glitch-soc and upstream and in particular helps uncovering unintended discrepancies
  • while I'm doing this, I'm finding dead code in both glitch-soc and upstream
  • once it's done, it should make following upstream much easier

Still, it's a lot of work, and I'm afraid this will also push a large amount of work on downstreams. cc @kescherCode @TheEssem

@kescherCode
Copy link

Well, we can't exactly cheat our way out of the upstream merge work, can we ^^ What must be done, must be done.

@ClearlyClaire ClearlyClaire force-pushed the glitch-soc/refactor/monolithic-components-css branch from b6b5aa9 to c48f3a4 Compare February 8, 2024 00:07
@ClearlyClaire ClearlyClaire force-pushed the glitch-soc/refactor/monolithic-components-css branch from c48f3a4 to 189b70c Compare February 8, 2024 17:50
@ClearlyClaire
Copy link
Author

Making some progress. I basically reordered rules, refactored things a bit, applied some upstream changes that were somehow missed previously, removed some unintended duplicates as well as some dead code… and also introduced dead code that we removed but upstream was still holding on (though I have removed them upstream, so we'll get to that in a later merge.

There are still significant differences, but most of them are due to actual feature/design/markup changes, so I'm considering either keeping them or tackling them in follow-up PRs.

Broadly, the changes I can think of are:

  • significantly different markup and layout for profiles: while I don't like everything in upstream's design, I think it's overall better-looking and more consistent than ours. I'm considering adopting it and working from there
  • a few glitch-soc small features that I'm not sure are worth keeping (e.g. the “sensitive” label on media attachments that are being shown)
  • significantly different markup and layout for statuses and composer

@ClearlyClaire ClearlyClaire changed the title [WiP] Merge SCSS in a single components.scss file Merge SCSS in a single components.scss file Feb 8, 2024
@ClearlyClaire ClearlyClaire marked this pull request as ready for review February 8, 2024 19:46
@ClearlyClaire ClearlyClaire force-pushed the glitch-soc/refactor/monolithic-components-css branch from c25a93e to d391e01 Compare February 8, 2024 20:13
@ClearlyClaire ClearlyClaire merged commit 084d051 into glitch-soc:main Feb 12, 2024
26 checks passed
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.

2 participants