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

instrument window's width is too small #2528

Closed
StCyr opened this issue Jan 29, 2016 · 99 comments
Closed

instrument window's width is too small #2528

StCyr opened this issue Jan 29, 2016 · 99 comments
Milestone

Comments

@StCyr
Copy link

StCyr commented Jan 29, 2016

Hello,

I'm creating this issue in order to keep track of the discussions held about this problem.

I originally reported this problem in issue in #2519. In that issue, it as been considered as a translation issue and has been solved accordingly.

Though, not everybody agree about the real cause of this problem (me being the first tbh).

Here are the opinions that were exchanged on the IRC about this problem so far:

(2016/01/27)
15:48:03 - StCyr: bah I'll send you an updated fr translation
15:48:27 - StCyr: but why aren't these windows resizeable?
15:49:16 - StCyr: I understand it wouldn't probably be very nice when resized, but hey what a nightmare to see this tab
15:49:51 - umcaruje: its a bad idea to make the window resizeable
15:50:12 - umcaruje: its up to the translator to use abbrevation to keep the width small
15:51:00 - umcaruje: bad design, maybe, but it most probably won't change
15:51:26 - StCyr: when you say "bad idea" you mean "would become ugly"?
15:51:37 - StCyr: (that's not such a bad idea)
15:51:50 - StCyr: (well at least I can understand it)
15:54:20 - umcaruje: well, what would happen to the instrument GUI?
15:54:36 - umcaruje: we'd need to add padding, and it would get really really ugly
15:55:21 - umcaruje: so yeah, if we actually had 2 rows
15:55:48 - umcaruje: with 3 items of the same, constant width, that'd be better tbh
15:57:51 - StCyr: that maybe an easier (and perfectly valid) fix
15:59:45 - StCyr: well not easier than updating the translation, but more robust
15:59:50 - umcaruje: yeah

...
(2016/01/28)
16:38:26 - StCyr1: should some developer wanna have a look: https://github.com/StCyr/lmms/tree/StCyr_2ndRow
16:38:43 - StCyr1: (for a better solution to #2519)
16:51:03 - grejppi: StCyr1: tbh adding a second row feels like only treating a symptom and not the cause which is the tiny size of instrument windows
16:52:06 - grejppi: the piano roll toolbar was recently split into two rows and it looks absolutely hideous
16:54:55 - umcaruje: grejppi: I'm the cause of that lol
16:55:18 - umcaruje: I said that instruments windows shouldn't be resizeable
16:55:32 - umcaruje: since of the fixed size that the artwork has
16:55:38 - grejppi: well it wouldn't make sense because of that
16:55:48 - umcaruje: yeah
16:55:58 - umcaruje: so I said, if we actually had 2 rows
16:56:11 - umcaruje: where we had 3 buttons of the same width
16:56:16 - umcaruje: that could fix the problem

16:56:49 - grejppi: tabs on two rows is never a good thing
16:57:29 - umcaruje: but its not a problem in french only
17:14:55 - umcaruje: so StCyr1 I built your branch
17:15:57 - umcaruje: I don't really think this is a solution
17:16:06 - umcaruje: after seeing the extra space it adds

17:16:23 - grejppi: shrink the text size then? GENIUS
17:18:21 - grejppi: using abbreviations would probably the most sane thing to do
17:30:17 - Babbage_: icons
17:34:27 - Babbage_: plug icon, envelope icon with a wave for on int, Fn, FX both common in the world, 5 pin mini plug, and something

@claell
Copy link
Contributor

claell commented Jan 29, 2016

How does other software handle this? Are the windows resizeable there? Options would be to scale some parts, to rearrange some icons etc.

@StCyr
Copy link
Author

StCyr commented Jan 29, 2016

Although, I don't want to continue arguing about this point, and I totally accept and respect grejpii and umcaruje's opinions, I would like to add some information to the discussion:

  1. Although my solution currently adds 20 pixels to the instrument views' height, the defintive solution should only add max. 11 pixels per row, and should ideally only create additionnal row(s) when needed (so, english locale wouldn't be impacted);
  2. We could also imagine other solutions like having arrows to scroll the tab bar, or make use of Qt's QTabBar widget which has a "edible" option so that tabs automatically shrink when there's not enough space to display them in their entirety
  3. I'm also not in favour of full translation (eg: "FX" should stay "FX", "BW" is perfectly understable by music savy french people, so is "pitch" and so on...)
  4. Regarding, the piano roll toolbar that now looks hideous to grejppi: Maybe the piano roll could automatically scroll so that the played note always gets displayed? But, first, how was it before? I mean what was the solution before to display such a large keyboard? Also, what's the purpose of this toolbar anyway? Do people really use it to play notes? If yes, well, I understand automatic scrolling isn't the ultimate answer.

@claell
Copy link
Contributor

claell commented Jan 29, 2016

Could it be an option to simply automatically reduce/increase padding adaptively to the length of words, so they exactly fit the space given? Also the font size could be lowered (adaptively to the needed size).

@StCyr
Copy link
Author

StCyr commented Jan 29, 2016

To reply to claell: I have no idea how other softwares do that.

My guess is that they also try to manage that with translation or icons as much as they can.

But, also, that they use several rows when their translations become so short that they don't make sense anymore

@StCyr
Copy link
Author

StCyr commented Jan 29, 2016

I think that padding is actually automatically adapted to the length of words (I'm not sure what you mean here): Tabs' width currently depends on tab's label (ie: a tab with a shorter label, like "FX", is shorter than a tab with a longer label like "ENV/LFO")

@StCyr
Copy link
Author

StCyr commented Jan 29, 2016

For reference, should I delete my branch, here's what I did so far regarding the automatic addition of rows:

--- a/src/gui/widgets/TabWidget.cpp
+++ b/src/gui/widgets/TabWidget.cpp
@@ -72,8 +72,8 @@ void TabWidget::addTab( QWidget * _w, const QString & _name, int _idx )
                }
        }
        m_widgets[_idx] = d;
-       _w->setFixedSize( width() - 4, height() - 14 );
-       _w->move( 2, 13 );
+       _w->setFixedSize( width() - 4, height() - 34 );
+       _w->move( 2, 33 );
        _w->hide();

        if( m_widgets.contains( m_activeTab ) )
@@ -108,20 +108,37 @@ void TabWidget::setActiveTab( int _idx )

 void TabWidget::mousePressEvent( QMouseEvent * _me )
 {
-       if( _me->y() > 1 && _me->y() < 13 )
+       if( _me->y() > 1 && _me->y() < 13 + m_tabheight )
        {
+                int twidth = 0;
+                int row = 1;
                int cx = ( ( m_caption == "" ) ? 4 : 14 ) +
                                        fontMetrics().width( m_caption );
+
+                // Parse the tab widgets to find out which one was clicked on
                for( widgetStack::iterator it = m_widgets.begin();
                                                it != m_widgets.end(); ++it )
                {
+                        // Switch to next row when we reach maximum width
+                        twidth += ( *it ).nwidth;
+                        if ( twidth >= width() )
+                        {
+                                cx = m_caption.isEmpty() ? 4 : 14 + fontMetrics().width( m_caption );
+                                row++;
+                                twidth = 0;
+                        }
+
                        if( _me->x() >= cx &&
-                                       _me->x() <= cx + ( *it ).nwidth )
+                                       _me->x() <= cx + ( *it ).nwidth &&
+                                        _me->y() >= 1 + m_tabheight * ( row - 1 ) &&
+                                        _me->y() <= 1 + m_tabheight * row )
+
                        {
                                setActiveTab( it.key() );
                                update();
                                return;
                        }
+
                        cx += ( *it ).nwidth;
                }
        }
@@ -169,7 +186,7 @@ void TabWidget::paintEvent( QPaintEvent * _pe )
        p.drawRect( 1, 1, width() - 3, height() - 3 );

        p.fillRect( 2, 2, width() - 4, m_tabheight, g );
-       p.drawLine( 2, m_tabheight + 2, width() - 3, m_tabheight + 2);
+       p.drawLine( 2, m_tabheight * 2 + 2, width() - 3, m_tabheight * 2 + 2);

        if( ! m_caption.isEmpty() )
        {
@@ -192,17 +209,31 @@ void TabWidget::paintEvent( QPaintEvent * _pe )
        }
        p.setPen( cap_col );

-
+        int twidth = 0;
+        int row = 1;
        for( widgetStack::iterator it = m_widgets.begin();
                                                it != m_widgets.end(); ++it )
        {
+                // Create a new row of tabs when there is no more space
+                twidth += ( *it ).nwidth;
+                if ( twidth >= width() )
+                {
+                        tab_x_offset = m_caption.isEmpty() ? 4 : 14 + fontMetrics().width( m_caption );
+                        row++;
+                        twidth = 0;
+                }
+
+                // highlight the active tab
                if( it.key() == m_activeTab )
                {
                        p.setPen( QColor( 32, 48, 64 ) );
-                       p.fillRect( tab_x_offset, 2, ( *it ).nwidth - 6, 10, cap_col );
+                       p.fillRect( tab_x_offset, 2 + m_tabheight * ( row - 1 ), ( *it ).nwidth - 6, 10, cap_col );
                }
-               p.drawText( tab_x_offset + 3, m_tabheight, ( *it ).name );
+
+                // draw tab's label
+               p.drawText( tab_x_offset + 3, m_tabheight * row , ( *it ).name );
                p.setPen( cap_col );
+
                tab_x_offset += ( *it ).nwidth;
        }
 }
diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp
index 545c82c..96fe01c 100644
--- a/src/tracks/InstrumentTrack.cpp
+++ b/src/tracks/InstrumentTrack.cpp
@@ -1407,7 +1407,7 @@ InstrumentTrackWindow::InstrumentTrackWindow( InstrumentTrackView * _itv ) :


        m_tabWidget = new TabWidget( "", this );
-       m_tabWidget->setFixedHeight( INSTRUMENT_HEIGHT + 10 );
+       m_tabWidget->setFixedHeight( INSTRUMENT_HEIGHT + 30 );


        // create tab-widgets
(END)
+       p.drawLine( 2, m_tabheight * 2 + 2, width() - 3, m_tabheight * 2 + 2);

        if( ! m_caption.isEmpty() )
        {
@@ -192,17 +209,31 @@ void TabWidget::paintEvent( QPaintEvent * _pe )
        }
        p.setPen( cap_col );

-
+        int twidth = 0;
+        int row = 1;
        for( widgetStack::iterator it = m_widgets.begin();
                                                it != m_widgets.end(); ++it )
        {
+                // Create a new row of tabs when there is no more space
+                twidth += ( *it ).nwidth;
+                if ( twidth >= width() )
+                {
+                        tab_x_offset = m_caption.isEmpty() ? 4 : 14 + fontMetrics().width( m_caption );
+                        row++;
+                        twidth = 0;
+                }
+
+                // highlight the active tab
                if( it.key() == m_activeTab )
                {
                        p.setPen( QColor( 32, 48, 64 ) );
-                       p.fillRect( tab_x_offset, 2, ( *it ).nwidth - 6, 10, cap_col );
+                       p.fillRect( tab_x_offset, 2 + m_tabheight * ( row - 1 ), ( *it ).nwidth - 6, 10, cap_col );
                }
-               p.drawText( tab_x_offset + 3, m_tabheight, ( *it ).name );
+
+                // draw tab's label
+               p.drawText( tab_x_offset + 3, m_tabheight * row , ( *it ).name );
                p.setPen( cap_col );
+
                tab_x_offset += ( *it ).nwidth;
        }
 }
diff --git a/src/tracks/InstrumentTrack.cpp b/src/tracks/InstrumentTrack.cpp
index 545c82c..96fe01c 100644
--- a/src/tracks/InstrumentTrack.cpp
+++ b/src/tracks/InstrumentTrack.cpp
@@ -1407,7 +1407,7 @@ InstrumentTrackWindow::InstrumentTrackWindow( InstrumentTrackView * _itv ) :

        m_tabWidget = new TabWidget( "", this );
-       m_tabWidget->setFixedHeight( INSTRUMENT_HEIGHT + 10 );
+       m_tabWidget->setFixedHeight( INSTRUMENT_HEIGHT + 30 );


        // create tab-widgets

@claell
Copy link
Contributor

claell commented Jan 29, 2016

With padding I meant the space between the words. It an still get a bit smaller. Also one could change it so that the whole space of the line gets used, even if the words are smaller. Currently in the English version there is some space on the right.

@BaraMGB
Copy link
Contributor

BaraMGB commented Jan 29, 2016

Resizing the instrument window is not possible at the moment. The most instruments use move() for layout their knobs and buttons. And the background is a bitmap. Perhaps we can double the width and show at the right side the envelope tab. Gradually we redesign the instruments to double width.

@Umcaruje
Copy link
Member

Perhaps we can double the width and show at the right side the envelope tab.

Yeah, that's another idea that comes to mind. If done well, it could be a winner.

@grejppi
Copy link
Contributor

grejppi commented Jan 29, 2016

Regarding, the piano roll toolbar that now looks hideous to grejppi: Maybe the piano roll could automatically scroll so that the played note always gets displayed? But, first, how was it before? I mean what was the solution before to display such a large keyboard? Also, what's the purpose of this toolbar anyway? Do people really use it to play notes? If yes, well, I understand automatic scrolling isn't the ultimate answer.

I was referring to this one:

kuvakaappaus - 2016-01-29 17-39-22

@tresf
Copy link
Member

tresf commented Jan 29, 2016

Overflowing tabs isn't a new concept. In regards to avoiding the translation, perhaps we entertain wordless artwork in its place and remove the text altogether.

@Spekular
Copy link
Member

Spekular commented Jan 30, 2016 via email

@StCyr
Copy link
Author

StCyr commented Feb 4, 2016

Hello,

I must say I'm not convinced at all by this idea of artwork.

First, I don't think wordless artworks would make this tab bar's height smaller than 2 rows of icons.
Secondly, I don't think it will be that easy to find meaningfull artworks for all the tabs.

Though, I would like to continue this discussion in the hope to find a solution to this overflowing tab bar.

So, here are some possible artwork ideas that I found on the Internet:

  • PLUGIN: Often represented as a piece of a puzzle, or a power plug. Maybe in this case it could be a house (meaning "home of the instrument window" )
  • ENV/LFO: Should be something like a sine or sawtooth wave
  • FX: I haven't found any drawing that directly make me think about "effects". But, maybe a simple artwork around the "FX" letters should be OK in any language.
  • MIDI: Usually represented as a keyboard, or by a MIDI (DIN 41524) connector
  • MISC: A star or a plus sign?

The other solutions evoked are:

  1. Doubling the intrument window's width (BaraMGB, Umcaruje);
  2. Using QTabBar widgets and use their "edible" property (StCyr);
  3. Change padding and font size (claell);
  4. Do not translate (StCyr);
  5. A second row of icons (StCyr).

Cheers,

Cyril

@Spekular
Copy link
Member

Spekular commented Feb 5, 2016

Maybe I'm wrong here but I don't think the icons need to exactly convey the
purpose of the tab. They should have some connection, but if were moving
away from text than we're moving from the only medium that can really
convey it. For that reason I think perhaps they should be more abstract,
for example:

  • Plugin: To me this is the generator, which is where we get the base
    signal. Therefore I'd use a circle or circle with turbine.
  • Env/Lfo: This is where the sound is shaped, so an envelope (not the kind
    you put letter in!) would make sense
  • FX: This one is tough, but an X would probably work
  • Midi: Keyboard or square
  • Misc: A plus

The only thing I'm not sure of is how to make these look good together, brb
with mockup.

@Spekular
Copy link
Member

Spekular commented Feb 5, 2016

Here are small/abstract icons:
new icons v1

They don't look incredible but I'd say that's down to my mockup skills (no antialiasing etc.) and not the concept. They could probably actually be drawn with a font/vectors which is an added bonus.

If we decide to have two rows we could also make the existing row taller and use bigger icons, which could better represent some of the icons in a less abstract way, similar to (but much smaller than) these:
ex1
ex2

@StCyr
Copy link
Author

StCyr commented Feb 5, 2016

I like your idea @Spekular

just simple symbols

... connected to (translated?) tooltips

... with the currently selected symbol highlighted

...

maybe the artwork fans will win this argument :-)

....

Hopefully their will be winners, and a solution will be implemented :-)

Cheers,

Cyrille

@tresf
Copy link
Member

tresf commented Feb 5, 2016

Here are small/abstract icons:

If we go this direction, I would advise we make the tabs slightly taller to accommodate a legible icon. We can still use the translation for a tooltip. Here are some placeholders. tagging @RebeccaDeField.

  • Plugin Tab: Puzzle piece
  • ENV/LFO: Wave (as @Spekular illustrated)
  • FUNC: Gear
  • FX: Mixer icon
  • MIDI: Plug or round midi icon

@RebeccaDeField
Copy link
Contributor

Hopefully, producers should be able to get an idea of what each icon is supposed to mean at a glance, so somthing literal and simple is always a good idea. I'm not a producer myself, so I can't judge what icons would make the most sense for this instance, but I'm sure you guys can work that out.

We will probably want a little more space on that bar for legibility.

I agree that using a tooltip is also a good idea.

@StCyr
Copy link
Author

StCyr commented Feb 12, 2016

Hi guys,

I just wanted to let you know that I've started a possible implementation of "artworkable" TabWidgets.

I've a working PoC here: https://github.com/StCyr/lmms/tree/StCyr_ArtworkTabs

Please tell me what you think about it.

Regards,

Cyrille

@StCyr
Copy link
Author

StCyr commented Feb 16, 2016

Hi,

I've updated my artwork tab PoC so that it can handle both pixmap and text tabs.

It works as follow:

1* When given a pixmapName, TabWidget will draw an artwork tab using "pixmapName"
2* When pixmapName is NULL, Tabwidget will draw a text tab using "_name"

PoC is here: https://github.com/StCyr/lmms/tree/StCyr_ArtworkTabs

It seems that I failed to properly compile my previous commit. It will compile properly now.

Please tell me what you think about it.

Regards,

Cyrille

@Spekular
Copy link
Member

Spekular commented Feb 18, 2016 via email

@StCyr
Copy link
Author

StCyr commented Feb 18, 2016

Cool,

The problem now is that I'm really not a design guy. So, creating new artworks might be terrible. But, I can try.

Anyway, I need new artworks to find out how to embed them in LMMS binary. So, I'll create (probably awful) new ones.

Any idea for a good icon's size?

Currently, the TabWidget for the "PLUGIN", "ENV/LFO", "FUNC",... tabs is 14 pixels tall, and 254 pixels large if I'm not mistaken. And, there are 6 tabs to be displayed.

Cyrille

@StCyr
Copy link
Author

StCyr commented Feb 18, 2016

Ok,

so I've given a try with 14x28 icons. Here there are:

midi_active
midi_inactive
plugin_active
plugin_inactive
fx_active
fx_inactive
env_lfo_active
env_lfo_inactive
miscellaneous_active
miscellaneous_inactive
functions_active
functions_inactive

PoC has been updated: https://github.com/StCyr/lmms/tree/StCyr_ArtworkTabs

PS: I told you they would be ugly ;-)

@claell
Copy link
Contributor

claell commented Feb 18, 2016

But they show the concept (and I don't think they are ugly). Maybe if this should get integrated into the main version, some designers will create some?

@tresf
Copy link
Member

tresf commented Feb 18, 2016

These are great. Mockup?

@RebeccaDeField
Copy link
Contributor

@StCyr Actually, that is a great start. Maybe you have more designer in you than you thought ;)

I wouldn't mind creating vector versions when I have a few minutes to spare, but I first need to know if the icons he attached make sense to everyone.

@StCyr
Copy link
Author

StCyr commented Feb 18, 2016

Hi,

Thanks for the compliment :-)

@tresf : I'm not sure I understand what you mean (french native here). If this is the source images that you want, I've created them with gimp and can attach them here tomorrow CEST. But, too bad, I didn't bother using 2 layers to ease rework :-/

I'll also try to add tooltips; I doesn't look too complicated (http://doc.qt.io/qt-4.8/qt-widgets-tooltips-example.html)

Cyrille

@tresf
Copy link
Member

tresf commented Feb 18, 2016

@tresf : I'm not sure I understand what you mean

Sorry, I meant a sample of these new icons in action (on the actual tabs on a sample instrument) 👍

@StCyr
Copy link
Author

StCyr commented Feb 18, 2016

Ah, ok.

Sure, I can take a snapshot tomorrow also.

Otherwise my branch https://github.com/StCyr/lmms/tree/StCyr_ArtworkTabs compiles and shows the artwork tabs in action :-)

C.

@StCyr
Copy link
Author

StCyr commented Feb 19, 2016

Here's a mockup where you can see TabWidget displaying both artwork and text tabs:

untitled

I'm up for improving them a little, but I guess I won't be able to do anything approching what @RebeccaDeField could do.

@BaraMGB
Copy link
Contributor

BaraMGB commented Feb 23, 2016

The thicker lined fits better in row.

@StCyr
Copy link
Author

StCyr commented Feb 23, 2016

I also prefer the thicker line

@midi-pascal
Copy link
Contributor

👍 for the thicker one.

@DeRobyJ
Copy link
Contributor

DeRobyJ commented Feb 23, 2016

👍 thicker one

@tresf
Copy link
Member

tresf commented Feb 23, 2016

👍 thick

1 similar comment
@zonkmachine
Copy link
Member

👍 thick

@RebeccaDeField
Copy link
Contributor

Well, I guess that has been decided then :) Here is the download link --> https://www.dropbox.com/s/trd1ombn4kws03e/TabIcons.svg?dl=0

Looking at the screenshots, I can see that the bar is 14px including the borders. This makes it so that the 12px icons overlap the borders. The bar will probably need to be a few pixels (I'm guessing 2px) higher to compensate.

@tresf
Copy link
Member

tresf commented Feb 24, 2016

This is waiting on a few items...

  1. Update artwork at Allow WidgetTab to use artwork tabs #2599
  2. Fix Travis build issues with our Qt5 builds

Once these items are completed, we'll be happy to merge and close this issue out.

@StakeoutPunch
Copy link

I believe it would make more sense if the plugin (instrument tab) icon was used for the FX plugin tab, and using the icon originally meant for the envelope tab for the instrument control tab. My reasoning is that the instrument tab controls the actual generation of the sound (sound wave/sine), while the FX tab adds plugins (puzzle piece) onto that source audio.

Mockup:
tco_mock_1

@claell
Copy link
Contributor

claell commented Feb 25, 2016

The white highlighting behind a selected tab still overlaps the tabbar about one pixel in height. Is this wanted?

@StCyr
Copy link
Author

StCyr commented Feb 25, 2016

@claell no i isn't wanted. Where do you think it overlaps? At the top of the tabbar, that's it?

@claell
Copy link
Contributor

claell commented Feb 25, 2016

If I zoom it to the highlighting behind the envelope icon the light grey space seems to overlap about 1px at the top and about 2px at the bottom.

@StCyr
Copy link
Author

StCyr commented Feb 25, 2016

Hi @claell

You're right.

I've fine tuned the positioning a little bit.

If we want to be picky there's still an issue. But, how hard is it to be solved?

Here's the latest version:

mockup4

@tresf I've fixed the QT5 compilation issue (and updated the artwork). Now, there might still be some fine tuning left.

@tresf
Copy link
Member

tresf commented Feb 25, 2016

I've fine tuned the positioning a little bit.

If no one is attached to that embossed background image, let's see what we can do with the larger tabs. Feel free to steal a pixel or two from the VOL/PAN/PITCH area. 👍 I'd even go as far as to ask @Umcaruje or @RebeccaDeField to assist here.

@tresf I've fixed the QT5 compilation issue

👍

@Umcaruje
Copy link
Member

I'll be glad to asist. I vote we get rid of the gradient. Gradients on such small surfaces take the clarity off.

@claell
Copy link
Contributor

claell commented Feb 26, 2016

@StCyr Now it looks like the whole tab got smaller? At least some icons are overlapping it. But maybe this was already the case before the cange. I am also for increasing the tab size.

And @Umcaruje if you think that gradients are not good on small surfaces, might it make sense to remove them from all those surfaces? Would need some rework of design, maybe one should give lmms a complete overhaul designwise.

@StCyr
Copy link
Author

StCyr commented Feb 26, 2016

@claell I didn't change the tab's height yet; It was already the case.

Hmm, the idea of changing the tab 'size was actually a little bit invasive 'cause TabWidgets are used at some other places too. For the record, here's the list of files were they may be in use:

cyr@opennms-dev:~/lmms$ grep -Rl TabWidget src/
src/gui/widgets/InstrumentSoundShapingView.cpp
src/gui/widgets/TabWidget.cpp
src/gui/AudioDeviceSetupWidget.cpp
src/gui/ControllerConnectionDialog.cpp
src/gui/SetupDialog.cpp
src/gui/CMakeLists.txt
src/gui/MidiSetupWidget.cpp
src/gui/dialogs/about_dialog.ui
src/core/NotePlayHandle.cpp
src/tracks/InstrumentTrack.cpp

But, with my changes artwork tabs can be treated differently than text tabs. And, artworks tabs would currently only be used in "src/tracks/InstrumentTrack.cpp" (concern of this issue).

So, I guess I can change only this tab's height and also only remove gradient from this tab.

There was also this idea of @StakeoutPunch to change the icon for the plugin and fx tabs. I'm not in favor of changing the fx tab's icon as it has the same look as the "show/hide FX mixer" icon in LMMS' general toolbar. But, its reasoning for the plugin tab (the plugin tab is the sound source for the InstrumentTrack) actually makes sense. I'm not convinced though that the simple sine-wave icon is good enough. Earlier @Spekular also suggested an icon like the followings for this tab:

untitled

untitled

Looking for "turbine" icons on google also returns interesting results: https://www.google.com/search?q=turbine&source=lnms&tbm=isch&sa=X&ved=0ahUKEwiknt7ciZXLAhXH1xQKHTr6B-sQ_AUIBygB&biw=1680&bih=953#q=turbine&tbm=isch&tbs=isz:i

@StCyr
Copy link
Author

StCyr commented Feb 26, 2016

I've updated my branch.

TabWidgets with artwork tabs are now 17 pixels tall while TabWidgets with text tabs are kept to 14 pixels height.

@Umcaruje
Copy link
Member

Umcaruje commented Mar 6, 2016

And @Umcaruje if you think that gradients are not good on small surfaces, might it make sense to remove them from all those surfaces? Would need some rework of design, maybe one should give lmms a complete overhaul designwise.

That's what we're doing in #2587 ;) And yes, I'll look into flattening all of those line things.

@claell
Copy link
Contributor

claell commented Mar 6, 2016

@Umcaruje thanks for the link, did not know about this.

@tresf
Copy link
Member

tresf commented Apr 23, 2016

I think this was very close to be finished and the PR over at #2599 has some commits since the last discussion. @StCyr can we get an update on this?

@StCyr
Copy link
Author

StCyr commented Apr 26, 2016

Hello @tresf

Your last request (25 Feb. in this issue) was to investigate the possibilty of having a larger (ie: taller) TabWidget for this specific toolbar.

I made this in commits dccf9f4 and 7aad86e

In addition:

  • I've reverted the change I made in commit 64efd68 to fix issue MIDI tab not shown in instrument editor (translation issue) #2519 (commit fc15755)
  • I've added a tooltip functionality so that one can have additional meaningful text when hovering over a tab (commit 3f09440)
  • I've changed the ENV/LFO tab's artwork to a more standard ADSR graph as it had more proponents (see zonkmachine's comment of Feb. 22 in this issue for the discussion starting point on this particular topic). That's the purpose of commit 857b6ed
  • I've fixed a bug when a TabWidget with only one tab would generate a SIGFPE error (division by zero). That's the purpose of commit d9edef3

By re-reading the discussion, it seems that 2 other requests might have been raised:

  • Flatening the TabWidget / Remove the gradients: The highlighted tabs are flatened, though I didn't changed anything related to the non-highlighted tabs' gradient.
  • The artwork of the first tab (PLUGIN tab / jigsaw puzzle artwork) received some criticisms but no formal request.

Cheers,

Cyrille

@tresf
Copy link
Member

tresf commented Apr 26, 2016

@StCyr thanks for the detailed response. I'd like to get this merged in soon. Can you be so kind as to provide a final mockup so that we can discuss please?

@Umcaruje can you please help review this (tagging you since you've been administering the theme changes).

@Umcaruje
Copy link
Member

Umcaruje commented Apr 26, 2016

Reviewed the code, it looks ok to me, I made some remarks over magic numbers and the fact that the tab widget is not themeable using css, but one could argue that the themeablity is out of scope for that PR, but it would definitely be a nice thing to have.

I haven't tested the branch yet though, I'll try to get around it, but can't make any promises :)

But anyways, if those minor things get fixed, this looks good to be merged 👍

@Umcaruje
Copy link
Member

Oh and I just read that gradients are still there, I'll look into that too when I get around testing this.

@StCyr
Copy link
Author

StCyr commented Apr 27, 2016

Hello @Umcaruje, @tresf,

As I said, the highlighting of tabs doesn't have gradient. I'll have a look
how to remove all TabWidget gradients.

For what concerne the themeability, I don't have any idea how to do that.
If you have any documentation pointers, and this isn't too much work ;-),
I'll be glad to do it.

I'll look at your other remarks and address them eventually.

Cheers,

Cyrille

2016-04-26 23:32 GMT+02:00 Umcaruje [email protected]:

Oh and I just read that gradients are still there, I'll look into that too
when I get around testing this.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2528 (comment)

@tresf
Copy link
Member

tresf commented Apr 27, 2016

For what concerne the themeability, I don't have any idea how to do that.
If you have any documentation pointers, and this isn't too much work ;-)

Here's a good recent example of how @Umcaruje's been doing it... f787982

@Umcaruje
Copy link
Member

Closed via #3569

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