search

linuxmint / xapp

Cross-desktop libraries and common resources
GNU Lesser General Public License v3.0
126 stars 44 forks source link

Make XAppPreferencesWindow appearance more consistent with other dialogs #44

Closed TomaszGasior closed 5 years ago

TomaszGasior commented 6 years ago

This pull request makes XAppPreferencesWindow more consistent with GTK built-in dialogs and other applications dialogs. It adds margin, frame and makes buttons sizes proper. See screenshots (Adwaita, Arc, Numix as examples). I opened font chooser dialog for comparison.

Before: before_adwaita before_numix before_arc

After: after_adwaita after_numix after_arc

TomaszGasior commented 6 years ago

@JosephMcc

The use of the view style class here also doesn't look great when you embed other view widgets in the prefs.

I still think my changes are good. White color (in light themes) is used to indicate nested boxes of contents. Look at GTK's notebooks, treeviews. They are white. And using "view" CSS class makes XApps preferences window more consistent with this meaning.

What about the file chooser or color chooser?

After my changes XApp preferences window is more consistent with all GTK built-in dialogs, even with color chooser (button sizes, window margins). File chooser is only one exception. But this is bad GTK design decision that GTK's file chooser matches Nautilus even if the user uses the desktop other than GNOME. And GTK authors are not interested in fixing it. I am not sure all other desktops, dialogs, windows should be broken because GTK's file chooser appearance is broken.

The current style matches where we are moving with a couple of other things such as Mint Welcome.

You (Xapps suite authors) are advertising XApps as neutral, generic, traditional and cross-desktop. Is it true? If you want to keep XApps truly generic and cross-desktop you should not support only Mint and Cinnamon. If you are interested only in Mint and Cinnamon, please don't call you apps generic, cross-desktop and rename it from X to cinnamon-* — and things will be more clear.

On the other hand, if you are interested in supporting* other desktops you have to use general and common design. Please compare appearance of Thunar (XFCE file manager) preferences window with current Xapps window. https://i.imgur.com/kw9t1uL.png It's not consistent. Buttons have different sizes, the background color is not consistent too. There is a lack of margins.

My change makes XApps window more consistent in desktops other than Cinnamon, without any problems for Cinnamon users — they will see these neutral design principles in other apps.

(* – In this context in "support" word I mean about visual consistency.)

Keep in mind, that I don't require from Mint/Cinnamon people to work with other distros and desktop (such as I don't require from GNOME people to test GTK in desktops other than GNOME) but if you are promoting XApps as generic and cross-desktop, consistency with other desktops than you use should be important — with feedback from users of other desktops. Or you should drop "generic" label from this project.

clefebvre commented 5 years ago

@JosephMcc can you review this PR again?

I don't really see any difference in mintwelcome here.

PS: The scope isn't limited to our own projects, I don't think Joseph mentioned Mint to mean that but simply to illustrate his rationale. Xapp is cross-DE/cross-distro, there's no need to question that.

JosephMcc commented 5 years ago

Ok, I reviewed this again. I'm actually good with the changes. I have a slightly modified version ready to go that's up-to-date, fixes the merge conflicts, and a couple small code styling issues.

@clefebvre You wouldn't see any change in Mint Welcome since it doesn't use this dialog.

clefebvre commented 5 years ago

@JosephMcc ok merge when you get a chance.. we need xapp out before we can finalize cinnamon.

JosephMcc commented 5 years ago

Ok. I'm going to merge an updated PR that works with master here: https://github.com/linuxmint/xapps/pull/55

@TomaszGasior Thank you for the work you did here.

clefebvre commented 5 years ago

Thanks guys.