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

Main version #19

Open
robertpainsi opened this issue Feb 1, 2015 · 36 comments
Open

Main version #19

robertpainsi opened this issue Feb 1, 2015 · 36 comments

Comments

@robertpainsi
Copy link
Contributor

Hi!

Changing/Updating/Fixing things for these awesome themes always includes fixing it for every single theme the same way. Nobody wants to do that and it's also error-prone.

So how about creating a main/default version of all css? So each theme just imports the main theme and adapt settings which are necessary. The original creator already thought about that by moving all colors into a single file.

Since I'm not familiar with "themeing" at all (but with css) I have to ask if its possible or if there are any drawbacks?

@rbrito
Copy link
Owner

rbrito commented Feb 1, 2015

Hi, @robertpainsi.

This is something that I actually had in mind since I started this fork and I think that putting the common files under, say, a common directory would be the best thing to do and include the necessary files in the respective themes.

In the worst case, creating symlinks would be a 2nd choice, but I would really prefer to have a single common directory that works just as you thought about.

Regarding disadvantages, I think (but I can't know for sure) that the original author of the theme created the files multiple times to make it easier for users that only wanted one flavour of the themes.

I don't think that this is needed and, even if it is, we can always do this with scripts when preparing to release a new version (which may be something that end users might want to have).

Thanks for the suggestions,

Rogério.

@rbrito
Copy link
Owner

rbrito commented Feb 1, 2015

I guess that what I meant with the previous message is: go ahead and unify the files.

Just take care of putting the comments that are in the files and that are informative (such as gotchas etc.) in the common file. It is better to have them well-documented so that we do not forget.

@robertpainsi
Copy link
Contributor Author

Hi @rbrito!

I started the good old merge tool meld to compare the gtk-3.0 folders. In my opinion it's manageable.

But bringing the files to a common base is hard without any experience on how to manage multiple css files. Also heavy refactoring has to be done first.

What I can do, is, creating a common base by using a diff tool. Maybe it's the best step to begin with. I will ignore the gtk.css file at the beginning because these files are the hardest to merge.

The plan would be:

  • Use one gtk-3.0 theme as common base (which one would be best?).
  • Remove all the css source code from each theme and use the import statement to import the common base
  • Compare each original css file of a theme to the common base. If there is a difference, then I will remove the code from the common base and add the corresponding code from each theme to the corresponding file. The result is a common base which just shares the code which all themes have in common. Each "not" common code will be in the corresponding css files.

@robertpainsi
Copy link
Contributor Author

Darn, wrong button ^^

@rbrito rbrito added this to the 2.03.90 milestone Feb 3, 2015
@robertpainsi
Copy link
Contributor Author

First, just to clarify, I'm always talking about the css files in the gtk3.0 theme only.

So, I was filtering out the files which doesn't differ (man, I ❤️ the terminal). There are 17 files which all themes have in common. That's great! 😸 Before I continued, I had to check something. As I mentioned in the previous comment, I want to use the @import statement to import the common css file and add all the themes unique stuff below.

I start doing this for the gtk-widgets.css file and copied it from MediterraneanDark to the MediterraneanCommon folder. In the MediterraneanDark folder I removed all content from the gtk-widgets.css file and just added the line

@import url("../../MediterraneanCommon/gtk-3.0/gtk-widgets.css");

Now I’m facing a problem. All references to the assets in the css file will point to MediterraneanCommon/gtk-3.0/assets but they should point to the theme itself (e.g. MediterraneanDark/gtk-3.0/assets). Also the theme has some serious problems and is unusable.

I came up with a quick workaround but it’s not very ellegant. Creating a link

ln -s ../../MediterraneanCommon/gtk-3.0/gtk-widgets.css gtk-widgets.common.css

and just adding the code to the gtk-widgets.css

@import url("gtk-widgets.common.css");

will do the job (for now).

So by just using this code

@import url("gtk-widgets.common.css");

.menuitem GtkCalendar,
.menuitem GtkCalendar.button,
.menuitem GtkCalendar.header,
.menuitem GtkCalendar.view {
    color: #ff0000;
}

everything stays the same except the numbers in the calendar will be red.

I don’t prefer this solution at all. It’s seems like a hack. Maybe you know a better solution?

The changes I made can be inspected here:
https://github.com/robertpainsi/pkg-mediterranean-gtk-themes/compare/unifyTest

Summary:

  • Moved file MediterraneanDark/gtk-3.0/gtk-widgets.css to MediterraneanCommon/gtk-3.0/gtk-widgets.css
  • Created link MediterraneanDark/gtk-3.0/gtk-widgets.common.css which points to MediterraneanCommon/gtk-3.0/gtk-widgets.css
  • Added the code with the red calendar font to MediterraneanDark/gtk-3.0/gtk-widgets.css

Sorry for the long comment. Here is a 🍠 (9gag joke 😸)

@darvelo
Copy link
Contributor

darvelo commented Feb 4, 2015

@robertpainsi I'm not really familiar with GTK CSS, but it seems like it has variables like SASS does. Maybe instead of the hardcoded asset URLs, we can have a variable declared in MediterraneanDark's gtk-widgets.css, followed by an import that will use the variable? Like:

/* MediterraneanDark gtk-widgets.css */
@currentTheme: "MediterraneanDark";
@import url("../../MediterraneanCommon/gtk-3.0/gtk-widgets.css");
/* MediterraneanCommon gtk-widgets.css */
...
background-image: url("../" + @currentTheme + "/assets/background-noise-toolbar.png");
...

I don't know if variables can be "seen" within imports and concatenated this way in GTK CSS, but if so it may just work.

@dodev
Copy link

dodev commented Feb 4, 2015

Hey, guys!

I've just skimmed over this issue and I see you came to the same conclusion as I did. I haven't reviewed in deep the tools which you suggested, yet. For now I wanted to share with you my idea.

The first step to solving the "multiple-theme-one-structure" problem is implementing a build process with source tree, target descriptions and target files. A very simple builder can be written in node.js (if something similar doesn't exist on npm).
Second, a nice css preprocessor can be very handy in the process of writing the actual source files. Something like stylus. This tool has a massive arsenal of features, but for the task of keeping a structure of files in sync and well parametrized I think the most useful will be the variables, the operators and the require operation.a bunch of developer goodies like
The pre-processor and the builder should be loosely connected (i.e. using a documented interface). Third, the process of developing a theme needs some dev goodies like:

  • a "live-server" which will track the source files for changes and dynamically rebuild your working copy of the theme. This will make testing the output really fast and easy.
  • some kind of an inspector for the widgets' tree structure. I've seen such a tool for GTK+ but I don't remember the name of it.

So this is briefly how I see dev process for multi-variant themes. All questions, suggestions and critic are welcome.
I want to take a shot on implementing the builder and the dev-tools.

I want to apologize, if you have already discussed those things and I just spam, but really I haven't had the time to read and try out everything.

PS And sorry if I made mistakes in the brain puke - I'll try to rewrite it on a fresh head.

@darvelo
Copy link
Contributor

darvelo commented Feb 4, 2015

@dodev I am totally in love with css pre- and post-processors like SASS and Autoprefixer with an npm builder like Grunt or Gulp (gulp preferred as it uses streams and not disk io). My only concern is that users probably won't be familiar with build processes and the like. It's possible that the Mediterranean Theme's maintainer/contributors may provide a separate build directory or gzip for that, if they think the tradeoff works well. I'm happy either way.

Tools for live inspection I think can be found in the wiki of this project.

@robertpainsi
Copy link
Contributor Author

Awesome ideas guys! @darvelo @dodev

Maybe gtk-3.0 is already capable of sass http://worldofgnome.org/adwaita-gtk-theme-is-now-ported-to-sass/ ? Since SASS > CSS !!! this would be awesome! Also the css files have been removed and replaced by scss files in this commit https://git.gnome.org/browse/gnome-themes-standard/commit/?id=c10689c7b2dc1352e7b329f58c9f653e5682dca5

@robertpainsi robertpainsi mentioned this issue Feb 6, 2015
@robertpainsi
Copy link
Contributor Author

Sadly not 😿
Also neither string variables nor concatenations work.

@robertpainsi
Copy link
Contributor Author

I created a PR (only for inspection 😼) with the first results #30

Sorry if there is a lot to read but these steps and further processes are important! It would allow us to create a build system (some unifications still have to be done by hand yet) and then simplifying the code rule by rule by using a superior styling language like sass.

@robertpainsi
Copy link
Contributor Author

So, these are the last changes I added to #30 but using a new branch (easier to follow and compare). The changes can be inspected here.

robertpainsi/pkg-mediterranean-gtk-themes@robertpainsi:commonBase...commonBase2#diff-19ccc410ad4df63dcf8b2f3ad1e281aaR6

These changes still ignore issue #24 and #26 atm because I don't know yet which changes can be unified or are needed.

Sooo, feedback 😸
Tell me if I forgot something, but I think we are ready for a build system meow 😼 and get rid of those backup files and links.

@robertpainsi robertpainsi mentioned this issue Feb 7, 2015
@robertpainsi
Copy link
Contributor Author

Finally got rid of all the stuff which just has been added as workarounds. Since this is mergeable, I created a pull request.
#31

There are no further improvements to make without switching to a superior styling language and creating a real build system.

@robertpainsi
Copy link
Contributor Author

...and we finally have a sass version with a clean and real Makefile. Check it out! 😼
https://github.com/robertpainsi/pkg-mediterranean-gtk-themes/tree/sass

[Insert awesome party emoji here!]

Just run make and let the magic happen!!!
Create some symbolic links in the ~/.themes folder to the themes if you don't have them already.

The widget-factory throws no warnings/error. I also compared the master with the sass version directly. To be more precise, the widget factory and nautilus. I added both theme versions to the unity tweak tool and switched between each theme. Nothing changes! Not even a single pixel nor reformatting happens. So the master and sass versions produces the same view/output. (except in the Night/NightDarkest theme. See not related changes below)

Since we have a common base and a build process now, the next steps is sass refactoring. Maybe add a make parameter for each theme too because changing something and running the make file will build all themes which may take a few seconds (<5 secs on my laptop). Also fixing some issues and "Spread the word to enthusiast sites" #25 too would be awesome.

Not related changes:
I also converted tabs to four spaces and removed all multiple empty newlines. So there are just single empty newlines. Also removed all non ascii chars. And since there have been some undefined variables, I also defined them correctly which improved the nautilus location bar in the Night and NightDarkest themes. Still not perfect yet, but better then the master version!

Thanks to @rbrito @dodev and especially to @darvelo for all the support.

@darvelo
Copy link
Contributor

darvelo commented Feb 9, 2015

@robertpainsi This is amazing work! 👏 Now we definitely have a good foundation to build from.

Some thoughts:

  • I noticed a new file MediterraneanCommon/gtk-3.0/colors.scss. It's interesting because the current repo references some colors (like @notebook_tab_gradient_b) that aren't defined anywhere, but this new colors.scss file defines them. What was the process you used to get the color values?
  • I'm not bothered by the ~5s build time, but I tried to speed it up for us by using sass partials (renaming files to start with an underscore) and concatenating all the arguments to --update in the Makefile onto one line and just executing the sass command once. But there was no speedup. Oh well.

Either way, most of the hard work is now done. Great job!

@robertpainsi
Copy link
Contributor Author

What was the process you used to get the color values?

None, for all the colors in colors.scss 😨 My purpose was to give them extreme color values atm to immediately see them in the OS, get the related variable and correct them. So, for these colors I couldn't assign a correct color value yet since I didn't know which parts of the OS are effected. css itself changes undefined variables to white I guess or maybe ignores the property completely.

Btw: There were three colors I could define. https://github.com/robertpainsi/pkg-mediterranean-gtk-themes/blob/cad144ac32d3cec1cbc8d8138f44ffb7b1fc4994/MediterraneanNight/gtk-3.0/scss/gtk.scss#L43-L46 and I also changed https://github.com/robertpainsi/pkg-mediterranean-gtk-themes/blob/cad144ac32d3cec1cbc8d8138f44ffb7b1fc4994/MediterraneanNight/gtk-3.0/scss/gtk-widgets.scss#L7-L11 . But these are the only changes I made to all the themes after the sass migration and the changes are only in Night and NightDarkest. And there is already an issue #26 which will deal with that. So someone has to look over it again.

I'm not bothered by the ~5s build time, but I tried to speed it up for us by using sass partials...

That's great! Always wandered what these underscore thingy was 😸 sass also has an watch statement which maybe comes in handy too with those partials.

Either way, most of the hard work is now done. Great job!

No, not at all. That's the easy part because you don't have to think. Most has been done by terminal commands and regexp. Only a few things have been done by hand. Now refactoring all the scss files is the most time consuming stuff but with a common base we only have to deal with one version of a file.
I already found a tool csscss (http://zmoazeni.github.io/csscss/) which looks for duplicate scss code. Unfortunately it doesn't resolve the duplicate code by itself. 😿

Sy for the long post. Always writing too much 😼

@darvelo
Copy link
Contributor

darvelo commented Feb 10, 2015

Oh, I was under the impression that the MediterraneanCommon dir was the source of all the duplicate code?

@robertpainsi
Copy link
Contributor Author

k, I should have been more precise. 😸

The common dir is the source of all the duplicate code shared by all themes but the files in the common dir also have to be refactored. Since we can use the power of sass now, we can use inheritance, functions and variables which can define more than just colors to reduce the common scss files and complexity and also improve them overall.

Naming the current process 'refactor all common scss' would be a good name for that. 😼

I thought that the csscss tool would help for refactoring. If there is duplicate code in a common scss file, we define them once and let the corresponding selectors inherit them. But I'm completely new to sass, so I'm not sure if that's the best way to do the refactoring...

@robertpainsi
Copy link
Contributor Author

...and gtk 3.14 has a new awesome tool I found today. The gtk-inspector (https://github.com/rbrito/pkg-mediterranean-gtk-themes/wiki/Tools#gtkinspector). I installed ubuntu 15.04 to test it out. If you do so too, you may have to fix a critical bug by hand in the current release https://bugs.launchpad.net/ubuntu/+source/casper/+bug/1415586

If you run something else than ubuntu, you maybe have gtk 3.14 already. Check it by using the command apt-cache policy libgtk2.0-0 libgtk-3-0

I also executed export GTK_DEBUG=interactive since the shortcuts for the inspector didn't work in the virtualbox.

The tool let you inspect all the gtk stuff. You can also add custom css code to test the changes in realtime and changing the themes which will only effect the current inspected window. AWESOME²

See the inspector live in action https://mclasen.fedorapeople.org/inspector.webm

The only thing we have to bring back in sass are css-variables since you can use them in the inspector. Numix already has tried to do that, but it's not pleasing me at all. The variables there are defined at the beginning but never used at all because sass replaces variables with values...

So, to use the inspector more efficiently, we have to prevent sass to replace sass-variables by its value but replace sass-variables by gtk/css-variables.

Maybe some trick like with the gtk shade function which is defined in _functions.scss will do it?

@robertpainsi
Copy link
Contributor Author

This is one way to do prevent the variables to be replaced with values:
scss:

@function var($name) {
    @return unquote("#{$name}");
}

@define-color color #f00;
$color: var("@color");

body {
    color: shade($color, 1.2);
}

css:

@define-color color #f00;
body {
  color: shade(@color, 1.2); }

But I don't like the redundancy and it's also error prone. I also don't want to drop this feature unlike the Adwaita theming team did. (http://comments.gmane.org/gmane.comp.gnome.gtk+.devel.general/24441).

Bullsh*t: The Adwaita and Numix team already do it the right way. My fault, I will add all the @define-color stuff like the other teams did. Nevertheless this doesn't solve the redundancy.

@darvelo
Copy link
Contributor

darvelo commented Feb 10, 2015

@robertpainsi Thanks for adding that tool to the wiki. That will help debugging a lot.

I think you're right about using a color function for debugging. I've done some work figuring it out. You can define a Custom Sass function in Ruby and use it with a global var and an input string like so:

$debug: false;

$button-color: #00F;

@function theme-color($color-name) {
    @return if($debug, unquote("@#{$color-name}"), themeColor($color-name));
}

.blue {
    background-color: theme-color(button-color)
}

Then have a custom function that will output the actual variable value:

# file: themeColor.rb

module Sass::Script::Functions
  def themeColor(colorname)
    assert_type colorname, :String
    environment.caller.var(colorname.value)
  end
  declare :themeColor, [:string]
end

Now when you call sass, make sure to import the custom function:

$ sass -r ./themeColor.rb sassFile.scss

Output when $debug is false:

.blue { background-color: #00F; }

Output when $debug is true:

.blue { background-color: @button-color; }

@robertpainsi
Copy link
Contributor Author

Oh, I was updating my previous post the same time you posted your previous comment. sorry! 😸

I changed my mind about the usefulness of showing those variable names instead of color values in the css files. I was always thinking about using the inspector. Because you can change colors and stuff live, you definitely need those defined color variables. But in the css code of the theme itself, it isn't important at all. I would also say that the real color code is more important because you can pick the color with some color picker and search it in the css file.

So, we should define the colors like here https://github.com/shimmerproject/Numix/blob/sass/gtk-3.0/scss/_colors.scss, so we can use them in the inspector, but don't use gtk variable names anywhere else.

Imo I don't see any need for this feature anymore.
But what do you think? Is there still any advantage to have the variables names instead of color values?

@darvelo
Copy link
Contributor

darvelo commented Feb 10, 2015

You're right. 😄 I think the way Numix does it is best.

@robertpainsi
Copy link
Contributor Author

k, sy for the circumstances! But it's good to see that sass could handle something like that!

Since you are already into the color sass/gtk variable stuff now, maybe you have an idea to fix the redundancy? As you can see, the variables are declared two times. Once as sass variables and once as gtk-variables.

$color: #f00;
@define-color color #{"" + $color};

It would be awesome if we could merge this in some kind of oneliner.

But I'm not sure if it's worth it and maybe it's a stupid idea again...

@darvelo
Copy link
Contributor

darvelo commented Feb 12, 2015

I think something like that would have to be done in Ruby. I really don't know much Ruby at all, but it would probably work similarly to what I posted. Hypothetically, you'd have a Sass function taking a name and a color, and passing them to Ruby to alter the Sass environment at a lower level.

It's not really a supported feature, and kind of hacky, so the redundancy might actually be the cleaner way, ironically. 😆

@robertpainsi
Copy link
Contributor Author

Thanks @darvelo ! The cleaner way it is. 😸

For now, I will ease back the throttle (slow down). Everything has been prepared for a big sass refactoring process. Also I'm not very familiar with sass, I'll leave further decisions to sass-experts. I add some notes I took during the common version process here. Maybe I'll create issues for each point. Nevertheless, the current step is luring more sass/gtk people to this repository. 😼

  • I have taken the _functions.scss file from https://github.com/shimmerproject/Numix/blob/sass/gtk-3.0/scss/_functions.scss and adapted it. Especially shade (sass also has a shade function) and the mixin border (this does a lot of things I'm not aware of!). We have to check these function!
  • Create folder/file structure similar to Ambiance #29 (comment) I'm not sure if it's easier to do the splitting of files now and refactor the sass code later or the other way round. There is a lot of code in gtk-widgets.scss and maybe it's easier to find duplicates and refactoring points if all the code is in one place.
  • All common files share a lot of code too. Maybe http://zmoazeni.github.io/csscss/ comes in handy
  • Correct me if I'm wrong, but as far as I've inspected the code, each theme specific code can be moved to the common one by just using some vars or if/else logic. For example, if the background is dark, use the light assets, else use the dark assets. I've seen that already in the Numix theme.
  • Some themes import the unity-greeter.scss some don't.
  • The import order shouldn't matter.
  • Improve makefile by some useful parameters 😸

@darvelo
Copy link
Contributor

darvelo commented Feb 12, 2015

@robertpainsi Thanks so much for the work you've put in so far. It'll go a long way to making this theme as current as possible.

Those are all good notes to keep in mind for the future. If you could make a separate issue and copy them there, it would allow others to easily follow the current state of the project.

It's a very interesting point you made about condensing everything into MediterraneanCommon and just using different variables for each derivative. It's a real insight that all themes might be reduced to a single scss file with their own variable values for colors and asset pointers!

@robertpainsi
Copy link
Contributor Author

Btw: I'm just taking a few days off for now but will continue working on this repo next week again. With less spare time on my hand, the next improvements/fixes I'll make will take much much longer.

I have written all the issues, as you suggested, but haven't posted them yet. Since the changes haven't been merged into the master yet and there is no other branch in this repo to link to. @rbrito has to do that as mentioned in #29 (comment) and #29 (comment)

@darvelo
Copy link
Contributor

darvelo commented Feb 19, 2015

@robertpainsi 👍

@rbrito, it might be time to merge the sass branch. It looks okay to me so far.

@robertpainsi
Copy link
Contributor Author

ping @darvelo @rbrito

How do we proceed? Imo it's mergeable into the master too. I'll create a PR #33 😼 If it's merged, we can start getting more users and contributors! 🎉

The only thing left is how to provide the themes?
At the moment you need sass to compile it. So you have to install ruby to get the gem package manager and to install sass. sudo apt-get install ruby && sudo gem install gem. Only then you can execute the make file.

Maybe we reuse the PPA to provide stable releases?

@rbrito : You should also consider to give some rights to other contributes. So we can support you with Issues, PR reviews and other maintaining stuff! 😸 Everybody here is professional and won't merge his own PRs.

@robertpainsi
Copy link
Contributor Author

Looking through the compare I found an error. Since I moved the files to a src folder but then back in the root again, all the xfm4 images which where defined in the .gitignores have been ignored.

I just wasn't aware of other .gitignore files than the main. 😾 I re-add the missing images and removed the unnecessary .gitignore files.

@robertpainsi
Copy link
Contributor Author

The sass branch is the branch I'm currently working on. I also did some unification without creating PRs so you guys couldn't review them. I'm sorry... That's bad practice. 😾

I createe a sass branch in this repo (https://github.com/rbrito/pkg-mediterranean-gtk-themes/tree/sass) and start creating PRs for this branch. We don't have to hurry to merge the branch into the master. 😼

@rbrito
Copy link
Owner

rbrito commented Mar 1, 2015

Hi, Robert.

On Feb 28 2015, Robert Painsi wrote:

The sass branch is the branch I'm currently working on. I also did some
unification without creating PRs so you guys couldn't review them. I'm
sorry... That's bad practice. 😾

The smaller, bite-sized the changes are, the sweeter. :) Oh, BTW, don't
forget to also send some commits to the README.md file, so that we know how
to build the themes.

Ideally (but that's for the future), we would like to have unit tests or
something that we could integrate with, say, Travis CI (like I do with other
projects of mine here: see https://github.com/coursera-dl/coursera).

That would be an extra, automatic layer of verification on what we humans
are generally bad at. :)

I'll create a sass branch in this repo too and start creating PRs for the
sass branch. We don't have to hurry to merge the branch into the master.

Is there something that we could sort-of release so as to get a wider
exposure and feedback? I have no doubts that people will have some oddball
application that we didn't even know that existed, but that will present
terrible results.

All this kind of disclaimer (e.g, which is the most tested theme, themes for
which we are looking for feedback etc.) should, IMVHO, go into the
documentation. What do you guys think?

Oh, I will be about 1 week and a half without internet access, which means
that I won't be able to see the great work and discussions that you guys are
having.

Thanks,

Rogério Brito : rbrito@{ime.usp.br,gmail.com} : GPG key 4096R/BCFCAAAA
http://cynic.cc/blog/ : github.com/rbrito : profiles.google.com/rbrito
DebianQA: http://qa.debian.org/developer.php?login=rbrito%40ime.usp.br

@robertpainsi
Copy link
Contributor Author

...also send some commits to the README.md...

On it! 😸

CI

Yeah, that would be awesome. The more automated it is the better.

Is there something that we could sort-of release so as to get a wider exposure and feedback?

I'm still working on some tickets which definitely should be done before the release. I hope that I can fix them before you come back 😸.

Oh, I will be about 1 week and a half without internet access,...

The nightmare of every computer scientist. 🙀

@robertpainsi
Copy link
Contributor Author

Oh, something important came up! I'll start working on the issues after 13th

@robertpainsi
Copy link
Contributor Author

And an important exam came up... It's one of my last ones... So, have to expand my pause to 30th. 😳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants