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

Title Bar Theming Support #2450

Closed
RebeccaDeField opened this issue Nov 19, 2015 · 97 comments
Closed

Title Bar Theming Support #2450

RebeccaDeField opened this issue Nov 19, 2015 · 97 comments

Comments

@RebeccaDeField
Copy link
Contributor

LMMS is built using Qt and and it uses Qt's QMdiArea (MDI stands for Multiple Document Interface). Every window inside LMMS is a QMdiSubwindow, and they are not themeable using CSS. Their look is hard-coded in the Qt settings for your PC

A solution someone mentioned would be to actually render frameless windows and then draw a custom titlebar on top of them.

I origanally added the issue here HDDigitizerMusic#5, but @Umcaruje suggested that I add this issue on the main issues tracker.

Edited 2015-11-18 @tresf, added screenshot
image

@tresf tresf added the gui label Nov 19, 2015
@tresf
Copy link
Member

tresf commented Nov 19, 2015

Duplicate of #123, which was consolidated into meta issue #1839.

We can leave this open, but we are in dire need of C++ coders, so unless there is movement on this in the next few months, I will close this out and turn the "Modern and Sleek" thread into a meta-issue so we don't bulk up the tracker.

In regards to making our internal MDI-style windows borderless and adding our own themed title bars, close, minimize, maximize buttons, the task is rather trivial for a seasoned Qt GUI developer (or any good C++ coder for that matter), we just need some people to step up. 👍

@tresf tresf changed the title Research Ways To Change The Window Borders Title Bar Theming Support Nov 19, 2015
@tresf tresf mentioned this issue Nov 19, 2015
5 tasks
@BaraMGB
Copy link
Contributor

BaraMGB commented Jan 24, 2016

I'm on it. But its not so trivial as @tresf said. But I think I can do this.

windowdec1

@Umcaruje
Copy link
Member

@BaraMGB I love you

@tresf
Copy link
Member

tresf commented Jan 24, 2016

^

@RebeccaDeField
Copy link
Contributor Author

@BaraMGB Nice work!! I think that if we can code up new title bars for the default, it should be more generic and clean so that I can easily match most of the themes.

Also, what @Umcaruje said.

@programnoir
Copy link

Hey there, popping in to say you're the man, @BaraMGB, this is a crucial feature that themers have been needing for a long time.

@BaraMGB
Copy link
Contributor

BaraMGB commented Jan 25, 2016

Okay, I got it. It was much more simpler as I thought at first. Since the Titel Bar belongs to the MDISubWindow widget, I only had to implement a paintEvent and draw the new Title Bar. Simple enough. The old Titel Bar is under the new and gives his functionality through the new one. So I don't need implement the whole window stuff such resize or the buttons. I only add new PNGs for close, minimize and maximize in the theme folder and load them on the right place. That's all.

But now I have to figure out the css stuff. I don't know how to implement that. But I will see.

windowdec2

@tresf
Copy link
Member

tresf commented Jan 25, 2016

now I have to figure out the css stuff. I don't know how to implement that. But I will see.

Here's how I did it in the grid patch... aaa798c. You may want to start a Pull Request now against your GitHub repo so that we can make advisement.

default/style.css

qproperty-foo: rgba( red, green, blue, alpha );

c++

Q_PROPERTY( QBrush foo READ foo WRITE setFoo )
QBrush foo() const;
void setFoo( const QBrush & c );
QBrush m_foo

@RebeccaDeField
Copy link
Contributor Author

@BaraMGB If you can give me the size of the icons for the title bars, I can design the minimize, maximize and close icons for you.

@programnoir
Copy link

I feel the need to point out the white space suddenly appearing underneath the scrollbars. The left border seems obscured too. I am out of the loop if this is from a different problem.

@programnoir
Copy link

Nevermind!!! Wrong screenshot!!

@tresf
Copy link
Member

tresf commented Jan 25, 2016

@BaraMGB If you can give me the size of the icons for the title bars, I can design the minimize, maximize and close icons for you.

@RebeccaDeField we may go vector. Do you agree that we may be able to tackle them all with a font + style sheet + css borders, etc? If so, we can skip the pixmaps for these particular buttons.

@RebeccaDeField
Copy link
Contributor Author

@tresf If we want to just code up the icons instead of using pixmaps, I could always just create a mockup to give the coders some direction for what it should look like. Let me know if that would be helpful :)

@BaraMGB
Copy link
Contributor

BaraMGB commented Jan 25, 2016

I think @tresf meant svg instead of pixmap.

@RebeccaDeField
Copy link
Contributor Author

@BaraMGB If so, I design everything in Inkscape so that would work as well.

Awaiting clarification from @tresf ;)

@tresf
Copy link
Member

tresf commented Jan 25, 2016

I think @tresf meant svg instead of pixmap.

AFAIK we've migrated almost no icons from PNG to SVG. Please correct me if this assumption is wrong in regards to the new theme.

@tresf
Copy link
Member

tresf commented Jan 25, 2016

@RebeccaDeField we may go vector

P.S. This statement was in regards to using pure CSS, not SVG (since a CSS border is a vector, not a pixmap). I realize now how confusing this statement is in the context of images. Sorry for the confusion that it has caused.

What I meant to say was, we should use fonts (which are vector) such as "X" to close, and a bordering box with CSS styling to decorate over an image of a close button.

@Umcaruje
Copy link
Member

Do you agree that we may be able to tackle them all with a font + style sheet + css borders, etc?

I think making an icon font and/or doing css hackery for just three icons is a bit of an overkill. Also, css hackery might be fun, but using pixmaps for the close, minimize and maximize icons will mean that users can easily change them in their themes, as opposed to CSS tricks, which require a lot more knowledge.

@tresf
Copy link
Member

tresf commented Jan 25, 2016

I think making an icon font and/or doing css hackery for just three icons is a bit of an overkill

Perhaps, but _ x in a styled box isn't rocket science either. 👍

@RebeccaDeField
Copy link
Contributor Author

Either way sounds great to me. Seems like @tresf could code it up pretty easily. Is there a specific reason that using css might better improve or speed up the program as opposed to just using the .pngs?

I also agree with @Umcaruje. We are low on coders so it might be a good idea to use something that can be easily updated.

Either way, I can't wait to see what it looks like in action. :)

@programnoir
Copy link

Actually, it's funny, but I managed to read the Qt Style Sheets reference, turns out QWidget::title is how you style the title bars of all of the widgets, albeit there seems to be a degree of limitations here and there. For example, I have yet to figure out how to change the text inside of the title bar, and it inexplicably seems to get a weird white background. The dialog buttons also disappear. In conclusion, really glad for BaraMGB's contribution.

@Umcaruje
Copy link
Member

screenshot from 2016-01-27 14 07 01

I fiddled around with @BaraMGB's branch, and must say I'm pretty happy.

Code info here: #2516 (comment)

@RebeccaDeField
Copy link
Contributor Author

Great work everyone! I think we should come up with a design for the title bars that matches the Default theme and put that in place 👍

Here is a link to the Title Bar icons I designed if anyone wants to grab them --> https://www.dropbox.com/home/LMMS%20Theme?preview=TitleBars.svg

@IvanMaldonado
Copy link
Contributor

This looks sweet! I can't wait to see this in action in the 1.2. This and the other contributions you have made about theming support will open a whole new branch of themes.

Thank you so much for doing this!

@Wallacoloo
Copy link
Member

It occurs to me that if we were able to have LMMS' subwindows make use of the system theme, then we could save ourselves a lot of effort in the long run - the user would be able to style LMMS just by opening up their OS' appearance configurator (e.g. Aero for windows systems, gnome-tweak-tool for gnome desktops, qtconfig for KDE systems, etc), and we'd gain the benefit that LMMS has a consistent look-and-feel with the rest of the applications on the user's system.

A quick experiment shows for me that lmms' subwindows (the border color, minimize/close icons, background color, font color, etc) don't seem to change look at all when I alter either my system's gtk theme or the qt4/5 theme, regardless of if I build LMMS with WANT_QT5 or not. I'm unsure if this is our own doing (are we explicitly overriding the system's theme in our code?) or a limitation of Qt5 and its subwindowing system. But I think it's worth looking into seeing if we can fix this on our end.

@BaraMGB
Copy link
Contributor

BaraMGB commented May 11, 2016

I'm a little bit busy in real life at the moment. I'll look at this at weekend. @tresf I'm shure we bring that thing to run for Mac, too 🍻

@BaraMGB
Copy link
Contributor

BaraMGB commented May 18, 2016

@tresf : Please test this branch: https://github.com/BaraMGB/lmms/tree/SubwindowDecoMac. I have implemented the suggestion of @liushuyu ( windowState().testFlag( Qt::WindowMaximized)). On Maximize the window it should be there an output on console: "maximize!". To implement this whole stuff on mac could be more difficult if that fails. That could be a problem.

@tresf
Copy link
Member

tresf commented May 18, 2016

I'm not sure what I'm testing, but the maximize button does nothing after it has been clicked once. It maximizes, never switches to restore and never works after that. There is nothing in the console that says maximize!.

Edit: I did clone and build the SubwindowDecoMac branch for these results.

@liushuyu
Copy link
Member

@tresf Then this meant that the condition windowState().testFlag( Qt::WindowMaximized) never turned True, which means Qt cannot handle the window status under Mac... That's really unfortunate

@tresf
Copy link
Member

tresf commented May 19, 2016

Is there a reason we've no changeEvent handlers? Am I missing something fundamental here? Why are we using the constructor to detect a flag?

@liushuyu
Copy link
Member

liushuyu commented May 19, 2016

Hmm... I've tried the changeEvent() , but it proved to break everything.
Somehow the window itself will misbehave when I even only add the dummy
changeEvent() (empty function) . And as for calling the constructor, that
was a hack which ran just fine on Linux and Windows, however, I was
disappointed when you told me this still didn't work.

@tresf
Copy link
Member

tresf commented May 19, 2016

I've tried the changeEvent() , but it proved to break everything.

In your tests, did you re-fire the super-class method? (Does C++ do this automatically on override or do you lose it without explicit super-class invocation?)

Edit: @liushuyu yeah, from what I'm reading, this would be an explicit override and would require calling QWidget::changeEvent(event); somewhere in the function as to reimplement the now overridden behavior.

http://www.qtcentre.org/archive/index.php/t-48140.html

@liushuyu
Copy link
Member

liushuyu commented May 19, 2016 via email

@tresf
Copy link
Member

tresf commented May 19, 2016

But the problem here is Qt couldn't handle this kind of event on Mac , as
it don't know whether the Window status changed when Maximized.

I'm not sure I see any evidence of that in the previous code examples. Checking on constructor is going to trigger false every time, no?

@liushuyu
Copy link
Member

liushuyu commented May 19, 2016

I'm not sure I see any evidence of that in the previous code examples. Checking on constructor is going to trigger false every time, no?

Hmm, in fact, changeEvent and resizeEventboth are working on Linux and Windows ... In addition Qt::WindowMaximized is a constant (value=0x00000002) not a constructor. And I tested this trick before on Linux (my laptop) and Windows (my friend's) , this just worked

Anyway, thanks for your hard work, intensive reading and testing!!! 👍

@BaraMGB
Copy link
Contributor

BaraMGB commented May 19, 2016

@tresf it seems, Qt don't really maximize the subwindow on Mac. It will only resized to the max. Therefore the flags, isMaximized and Qt::WindowMaximized, return false.

I have to write a workaround now for this. But this is much more difficult. Because I have to implement new slots, new double click event and new context menu. We have to proof all maximize order go thru our own slot to set a own flag.

Unfortunately that doesn't runs on non Mac systems, because there we have no chance to connect the restore button with our own slot.

I do my best to make the code as simple as possible.

@tresf
Copy link
Member

tresf commented May 19, 2016

I'm not convinced as restore works just fine from the context menu.

@BaraMGB
Copy link
Contributor

BaraMGB commented May 19, 2016

Yes, but we have to know if the user restores the window for change the buttons. The context menu don't tell us. So we have to implement an own context menu.

@tresf
Copy link
Member

tresf commented May 19, 2016

Yes, but we have to know if the user restores the window for change the buttons. The context menu don't tell us. So we have to implement an own context menu.

I feel we're over-complicating this. To state there's no way to know that the context menu has been used I feel to be a false statement. Furthermore, to design this around the context menu I find to be a bad idea as well. We should detect the resize event and act accordingly.

@BaraMGB
Copy link
Contributor

BaraMGB commented May 24, 2016

@tresf Okay, I've figured out an other way, perhaps. But I need your help again. If we check the size of the subwindow in relation to the MdiArea, so there is a way to check if the subwindow is maximized. I have add a qDebug() which gives us the size of both. My plea to you today is to start this one, maximize the songeditor and paste the console output here. I can write a little function to test if the subwindow is maximized on mac and all is fine.

Here is the branch: https://github.com/BaraMGB/lmms/tree/fixMacSubwindow

Thank you for your efforts.

@tresf
Copy link
Member

tresf commented May 25, 2016

Output:

Area size:  QSize(655, 448) Window size:  QSize(543, 335) 
Area size:  QSize(655, 448) Window size:  QSize(400, 200) 
Area size:  QSize(655, 448) Window size:  QSize(600, 300) 
Area size:  QSize(997, 591) Window size:  QSize(983, 577) 
Area size:  QSize(997, 591) Window size:  QSize(997, 591) 
Area size:  QSize(997, 591) Window size:  QSize(600, 300) 

@liushuyu
Copy link
Member

liushuyu commented May 25, 2016

Area size:  QSize(655, 448) Window size:  QSize(543, 335) 
Area size:  QSize(655, 448) Window size:  QSize(400, 200) 
Area size:  QSize(655, 448) Window size:  QSize(600, 300) 
Area size:  QSize(997, 591) Window size:  QSize(983, 577) 
Area size:  QSize(997, 591) Window size:  QSize(997, 591) 
Area size:  QSize(997, 591) Window size:  QSize(600, 300) 

@tresf For what I see, it seems that you only maximized the window only once (the 6th 5th value), is that correct?

If that is correct, then this solution would probably work

@tresf
Copy link
Member

tresf commented May 25, 2016

@tresf For what I see, it seems that you only maximized the window only once (the 6th value), is that correct?

I maximized the song editor and then had to use the context menu to restore it. I don't know which line is which, but at a glance, I'd assume it's the 5th where Area size = Window size

@liushuyu
Copy link
Member

I maximized the song editor and then had to use the context menu to restore it. I don't know which line is which, but at a glance, I'd assume it's the 5th where Area size = Window size

Then, I assume that means this will ... work ... ? (Finally...)

@tresf
Copy link
Member

tresf commented May 25, 2016

Then, I assume that means this will ... work ... ? (Finally...)

Yeah. I just tried it again. Although I don't entirely trust the 4th dimensions much (shouldn't they be identical to the 6th?) I can confirm the 5th line is what happens when I maximize.

@BaraMGB
Copy link
Contributor

BaraMGB commented May 25, 2016

I think we got a winner. The window size is identical to the MdiArea size. 👍

@BaraMGB
Copy link
Contributor

BaraMGB commented May 25, 2016

@tresf Okay, I've updated the branch. I cross my fingers but this should fix the issue.

@tresf
Copy link
Member

tresf commented May 25, 2016

Success. Tested with SongEditor and PianoRollEditor.

image

@liushuyu
Copy link
Member

Success. Tested with SongEditor and PianoRollEditor.

That's amazing!! Thanks @tresf @BaraMGB for your hard work!! Congratulations!!

@BaraMGB
Copy link
Contributor

BaraMGB commented May 25, 2016

Great! I'll make a pull request tonight.

@tresf
Copy link
Member

tresf commented Jun 23, 2016

Great! I'll make a pull request tonight.

Merged, thanks!

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

8 participants