ilia3101 / MLV-App

All in one MLV processing app.
https://mlv.app/
GNU General Public License v3.0
283 stars 29 forks source link

Scroll bar cover buttons #41

Closed seescho closed 6 years ago

seescho commented 6 years ago

MLV-App Standard style: The scroll bar covers the very right part of the expanded tabs. darkstyle is OK.

masc4ii commented 6 years ago

Really? In my Ubuntu it looks good... pic

masc4ii commented 6 years ago

May it depend on the system style or font size?

seescho commented 6 years ago

Lokks like this

pic

masc4ii commented 6 years ago

Not nice, I agree. Looks like the fonts are to big. But we fixed the size. So it may be the font type. If this one is to wide, the space may be not enough. But on the other side - also for DarkStyle we don't adjust the font type. So why DarkStyle is okay and Standard is not?! Does this also happen, if you change the Linux Style or Fonts?

seescho commented 6 years ago

Not sure, where I have to change what to answer your question, sorry.

masc4ii commented 6 years ago

Can't tell you how exactly you can do that. On Ubuntu there are many possiblilties to adjust really everything - fonts, font sizes, themes, etc...

seescho commented 6 years ago

The only effect in taking a smaller font size was a smaller font size. The problem remained. So I played a bit with MainWindow.ui.

I changed minimumSize (Breite) in both, docWidgetEdit and docWidgetContents to 240 and compiled that stuff. Now it looks fine for me.

bouncyball-git commented 6 years ago

Now It looks good in light/standard style or it look nice in both light and dark?

masc4ii commented 6 years ago

I'll do something what only affects Linux Standardstyle ;-)

masc4ii commented 6 years ago

Could you test please?

seescho commented 6 years ago

Yes, the cover problem is fixed for me.

It´s interesting, that this change didn´t affect the darkstyle at all. Darkstyle looks like before the change (was OK before and is OK now).

There is one more thing, I noticed:

Darkstyle: Details: sharpen and Chroma blur radius. The "0" doesn´t fit fully into the slider fields. I can only see a half "0".

the change fixed that for standard style. In darkstyle the problem remains: a half-cutted "0".

masc4ii commented 6 years ago

Yes, that was intended to only change the bahaviour for Linux standard. On Win/OSX Darkstyle/Standard was okay.

The "0" is half cutted also by the scrollbar? So for you it looks different than in this Video from Bouncyball? Very strange again.

masc4ii commented 6 years ago

One Parameter was slightly different for the sliders, which are aligned with the values. I changed that now. Could you please try?

seescho commented 6 years ago

No succes. Her a comparition between the two styles (in openSUSE Thunderbold)

pic

For my taste, darkstyle with it´s smaller buttons looks better than standard style

masc4ii commented 6 years ago

Agree, but that depends on the system style. What looks strange for my taste: the fonts are soooo big on your pictures. I think "Chroma Separation (...)" makes this error. If you delete (YCbCr) it could be right - this is too long. When that works, I will delete "(YCbCr)" (or add that to a tool tip).

seescho commented 6 years ago

Standard font on my system is Noto Sans 10.

seescho commented 6 years ago

I deleated (YCbCr). No change with the error. But I noticed another strong behavior, which (maybe) is related to this error. I will do some tests tomorrow and report then.

masc4ii commented 6 years ago

Okay... I commited this for you to test too. Hmmmmm, very stange.

seescho commented 6 years ago

The "#ifdef ... #elif definied " doesn´t run as expected. I changes MainWindow.cpp lines 758 ff. like this:

#ifdef Q_OS_MACX
    if( m_styleSelection == 1 )
        CDarkStyle::assign();
    else
        ui->scrollArea->setVerticalScrollBarPolicy( Qt::ScrollBarAsNeeded );
#endif

#ifdef Q_OS_LINUX
    ui->dockWidgetEdit->setMinimumWidth( 240 );
    ui->dockWidgetContents->setMinimumWidth( 240 );

    if( m_styleSelection == 1 )
        CDarkStyle::assign();
#endif

Now the issue is fixed for me. I hope, I wrote the code in a correct style. I´m just at page 200 of 1200 in my C++ learnig book :)

seescho commented 6 years ago

Wait. It was late yesterday night. I guess, this one will not run on windows.

Edit:

This one should run:

#ifdef Q_OS_LINUX
    ui->dockWidgetEdit->setMinimumWidth( 240 );
    ui->dockWidgetContents->setMinimumWidth( 240 );
#endif

    if( m_styleSelection == 1 )
        CDarkStyle::assign();
    else
        ui->scrollArea->setVerticalScrollBarPolicy( Qt::ScrollBarAsNeeded );

That´s because I need the minimumWidth (240) in both, standardstyle and darkstyle. Then the problems are fixed in both styles here in openSUSE. I hope, this runs in Ubuntu as well?

What is this line for?: ui->scrollArea->setVerticalScrollBarPolicy( Qt::ScrollBarAsNeeded ); Commenting it out makes no difference for me.

masc4ii commented 6 years ago

This could solve it for your Linux:

if( m_styleSelection == 1 ) CDarkStyle::assign();
#ifdef Q_OS_MACX
    else ui->scrollArea->setVerticalScrollBarPolicy( Qt::ScrollBarAsNeeded );
#endif
#ifdef Q_OS_LINUX
    ui->dockWidgetEdit->setMinimumWidth( 240 );
    ui->dockWidgetContents->setMinimumWidth( 240 );
#endif

But then, on all Linux and on both designs the sidebar has a bigger width. And if someone comes with a Linux with another font, the problem will be back.

#ifdef Q_OS_MACX
    else ui->scrollArea->setVerticalScrollBarPolicy( Qt::ScrollBarAsNeeded );
#endif

This line was for OSX only. OSX can have scrollbars which hide when not used. This would crash the design if so, without this line.

seescho commented 6 years ago

This could solve it for your Linux:

Works like a charm. Thank You.

And if someone comes with a Linux with another font, the problem will be back.

My system default font is Noto Sans 10. I give it a try with noto Sans 14, which is really big. Looks a bit ugly, but no problems with the sidebar.

This line was for OSX only.

Ahh, I see. I tried it in openSUSE and it worked too. But I prefer, not to hide the scrollbar.

masc4ii commented 6 years ago

Okay, then we let it like that for Linux. When it works, all is fine! ;-)