pragha-music-player / pragha

Pragha is a Lightweight Music Player for GNU/Linux.
GNU General Public License v3.0
173 stars 35 forks source link

Better compatibility with Gtk3 #87

Closed mgyugcha closed 8 years ago

mgyugcha commented 8 years ago

Hello, I made some changes in pragha for better integration with GTK3.

pragha-main-window

pragha-dark-window

pragha-header-bar

pragha-equalizer

pragha-open-file

pragha-preferences

pragha-xfce

Thanks for reviewing. I love pragha :)

matiasdelellis commented 8 years ago

Hi @mgyugcha Good job and thank you for helping me!.

Please you can make a new pull request for each change? It's easier to do the review, and discuss changes..

About changes, without test or see commits (They are very large and freze firefox. haha):

Added a GtkHeaderBar for adapt to new versions of Gnome and Gtk+.

It looks great, I just hope that respects the preference in the dialogue. :wink:

Added a new gboolean to pragha-preferences, for save the state of the dialog. If this gboolean is FALSE disables the equalizer and prevents user interaction with the interface.

Yes!, it was something that was pending .. :+1:

The minimum version of Gtk to compile 3.12

Why? I think it is unnecessary, but we can discuss it later. :pensive:

Spanish language has been updated.

No.. Use https://www.transifex.com/matias/Pragha/ to it and ignore any changes that have to do with translations in pull request!. :neutral_face:

Rework some dialogs (equalizer, open files, open locations...)

Again, +1, but optional.. :wink:

Now it used symbolic icons.

NOOOO. Sorry, but I will not accept this change!. Theme designers are able to force it .. :disappointed:

e.g: https://github.com/shimmerproject/Greybird/commit/61ec18d22780aa87998381599c941e0cf4f7bfb5

A more detailed description of my position here.. https://blogs.gnome.org/mclasen/2014/06/24/whats-that-icon/#comment-1211

Added margins to some containers (GtkBox, GtkPaned, GtkWindow).

Perhaps it would be useful, I have to do what they are..

Added Dark mode.

Wow, I guess it's okay. :wink:

Wow.. Thanks again!. Expect the new pull request and gladly merged.. :smile:

mgyugcha commented 8 years ago

Hello, the principal reason for use symbolic icons is that in Gnome Desktop the icons have a problem.

screenshot from 2015-10-14 15-34-20

Then, I have a solution. We can use symbolic icons only in Gnome and in XFCE use the default icons.

matiasdelellis commented 8 years ago

Hi,

Hello, the principal reason for use symbolic icons is that in Gnome Desktop the icons have a problem.

Exactly, therefore I insist that it is a problem of designers..

The problem is that gnome designers not have the corrects icons.. In the same way, there are others icon themes that do not have some the symbolic icons, but they have colored icons (See elementary icon theme).. and the problem is that designers must have complete icons set. both symbolic, as colored.

Sorry, but as a developer we should not worry about this.

Designers must complete their icon themes, and then select symbolic icons when strictly necessary.

p.s: See https://github.com/pragha-music-player/pragha/commit/b7ca1f1daae8acb51efe3896040aa212dfef783b In a moment try to fix it, but then discovered that the experience never is complete because in the end depends of designers..

mgyugcha commented 8 years ago

Ok. I'll put icons as they were. There is another problem to solve in this pull request?

matiasdelellis commented 8 years ago

Hi,

Ok. I'll put icons as they were.

Ok.. Sorry if you're not satisfied, but I think it's best..

There is another problem to solve in this pull request?

Mmm.. Your text editor is turning spaces in tabs automatically. (For example in function prototypes that you never change.), that generates many changes of lines, and therefore a huge patch. The problem is not the size, it is practically impossible to find your true changes between a so big diff.

Try that at the pull request just include your changes.. Nothing more. :wink:

Also other code styles.. like using spaces instead of tabs for spacing functions .. In general use:

{
|←tab→|{
|←tab→|←tab→|a_very_long_func_name_breaking_120_chars (type first_argument,
|←tab→|←tab→|← - - - any sane number of spaces - - - →|type second_argument,
|←tab→|←tab→|← - - - any sane number of spaces - - - →|type third_argument);
|←tab→|}
}

You're new contributing? I know these things are annoying, but not get discouraged!!. :smile:

Probably cost you a couple of attempts to do well, but it's worth .. :smirk: Saludos!!.

mgyugcha commented 8 years ago

Hi, I solved the issue with symbolic icons. Thank you for this great music player and sorry for my short words, but I don't speak much English.

I have more ideas for Pragha. Can you give me your email?

matiasdelellis commented 8 years ago

Hi!!.. Please, send me a mail (See my address on source code ;) and we speak in Spanish.. :wink:

mgyugcha commented 8 years ago

For display purposes the dark-mode has been removed.

matiasdelellis commented 8 years ago

Hi again, As I comment you in private, there are many accumulated commits..

Try doing a branch WIP/CSD, and add your changes of client side decorations in a single commit to see you're doing and to comment easier. :wink:

Then we can make a new pulls request and merge it. ;)