search

lunakurame / firefox-gnome-theme

A theme for Firefox 57+ matching GNOME Adwaita.
The Unlicense
200 stars 15 forks source link

New files structure #36

Closed rafaelmardojai closed 5 years ago

rafaelmardojai commented 6 years ago

New ui files organization proposal:

About the CSS Pre-Processor, we have some options to choose:

lunakurame commented 6 years ago

I was thinking about switching to Sass or Stylus before, but tbh I don't see many benefits in our case. We can ignore other browsers and just use the most recent CSS supported by Firefox, so we already have imports, variables, we don't need mixins and prefixes. Half of our selectors must match exactly Firefox' internal styles, so we can't change them. The other half isn't too nice either, because Firefox' DOM isn't nice in general.

Pros:

Cons:

So I don't think we have much reasons to switch to a preprocessor. What are your reasons? What features are we missing? If it's just about the fact that theme.css is too big, we can split it into multiple normal CSS files, it's not a problem sine we've got @imports.

rafaelmardojai commented 6 years ago

You have a point about the CSS preprocessor, i give you the reason in that.

Now i think we should focus this discussion on new files structure. I was working on it, and this is my new proposal:

Let me know what you think. If you want i can start something in a branch of my fork.

ghost commented 6 years ago

Save for symbolic-tab-icons, I think all of those optionals belong in partials.

rafaelmardojai commented 6 years ago
  1. csd.css need to be optional since FF still allow titlebar option. (Is there some way to detect if CSD is enabled with CSS?)
  2. hide-single-tab.css and matching-autocomplete-width.css can be enabled by default, but we need to discuss that before, since some users may not want it.
  3. private-urlbar.css is a small tweak to UX. IMO should be enabled by default.
lunakurame commented 6 years ago

That folder tree you posted looks good, but we can't adopt it as is, because I'd like to minimize breakage. We added a bunch of features to make it easy to update the theme. I added @imports for customChrome.css and customContent.css, so user config can survive updates and users can add their own styles, that feature was requested in #14. Those two files were then replaced by new installation instructions, so the theme doesn't conflict with other styles users might have installed: #18, but I didn't remove the old custom*.css files, because some people still use them. In our current state, in most cases just running git pull is enough to update the theme and everything should work according to your config after the update (except for experimental features like FF 60 support).

I don't know exactly how many people use this theme, but on average we have about 47 unique cloners per week, this theme exists since 2017-12-01, so I estimate we have around 900 users. If we move / rename files imported in the user*.css files, the theme is gonna break for all of them. That's why all files in your themes and optionals folders must remain in the ui folder and the pages folder can't be renamed. So that leaves just the theme.css file, it can be moved / renamed / split into multiple files, because it's not imported by end users directly.

I think breaking theme.css into multiple files, like your partials dir is a good idea (although I would probably name it parts instead), I would also move the core.css file to that folder, so all files directly in the ui folder could be imported by the users and all files in the parts dir would be internal stuff users shouldn't import separately.

lunakurame commented 6 years ago

Answering your last post:

  1. It was possible to detect CSD with the old Fedora patch, smithfred has a bunch of code for CSD auto-detection: https://github.com/smithfred/firefox-gnome-theme-3.26-layer, but he told me it doesn't work anymore, so currently we can't detect CSD.
  2. I wouldn't enable those by default. hide-single-tab.css makes it impossible to mute tab if there is only one tab and matching-autocomplete-width.css makes the autocomplete menu significantly smaller on lower resolutions and has a bug on 4K displays. Those are minor issues and don't cause much problems, but it might be annoying for some. Since there is no way to "un-import" in CSS, all extra features must be opt-in, not opt-out.
  3. Okay, in the case of private-urlbar.css I can agree it can be enabled by default. I see no reason why someone would not want to enable it. It doesn't break anything and the difference between normal and private windows is clearly visible. I would say it's even more visible than Firefox' stock behavior.
rafaelmardojai commented 6 years ago

So the result should be something like this:

lunakurame commented 6 years ago

Yeah, looks good. Those 3 files will stay in the ui folder too since we can't detect CSD: csd-1-button.css, csd-2-button.css, csd-3-button.css. And those files in the parts dir might turn out to be different, but that's gonna be clear when we start splitting the theme.css file into smaller ones (depending on what makes more sense, eg. I'd move the headerbar to headerbar.css, we might also separate specific features if that helps, analogously to private-urlbar.css).

ghost commented 6 years ago

makes it impossible to mute tab if there is only one tab

Ctrl + M always works and, considering the tab you want to mute is already open, it's also faster than right clicking the tab. I agree on the matching-autocomplete-width.css, though, best not enable by default if it's buggy.

lunakurame commented 6 years ago

Thanks, I didn't know about that key shortcut. You can also click the speaker icon instead of right clicking, that's as fast as Ctrl+M. Still I think key shortcuts are too obscure to justify removing a UI element (unless it's optional like it is now).

Alternatively we could add a file ui/recommended-features.css, it would be something like "just give me the most GNOME-like experience", so currently it would be equivalent to:

@import "ui/hide-single-tab.css";
@import "ui/private-urlbar.css"; /* I guess this one will be moved to the core? */
@import "ui/matching-autocomplete-width.css";

And when we add new optional features, we can also add them to that file. So it gives the users an option to enable all those features we would want to enable by default, but we can't because there is no opt-out in CSS. It will also automatically enable features we add in the future without user intervention while users who don't want those "recommended features" still have an option to just not enable it and customize their setup manually like they do it now. What do you think?

This won't include the theme variant and CSD since we can't detect those, just the optional features which would be nice to have enabled by default.

rafaelmardojai commented 6 years ago

I agree about adding a new file for fast enabling those features. Also if we find a way to auto detect CSD, i think we should move that feature to the core.

Yeah, private-urlbar.css should be on core.

lunakurame commented 6 years ago

I started splitting theme.css into parts: df37a7af8160fadb730bd87c7c13334e804f9b95, more coming soon. You were right, the current code is confusing and I'm sorry you had to work with it. :p I'm trying to write more consistent comments this time and split it into meaningful categories, so I hope the new parts are better.

rafaelmardojai commented 6 years ago

Firefox 61 has been released and auto CSD is working nice, probably is time to put it on core.