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

Deere :: compact FX units #1909

Merged
merged 11 commits into from
Dec 26, 2018
Merged

Deere :: compact FX units #1909

merged 11 commits into from
Dec 26, 2018

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Nov 19, 2018

This is an attempt to make the FX units a little more compact.
It's not polished yet. What do you think?

before:
deere-compactfx1_before

.
now:
deere-compactfx1_now
deere-compactfx2_now

@daschuer
Copy link
Member

Nice idea.

@ronso0
Copy link
Member Author

ronso0 commented Nov 20, 2018

expand toggle moved to the right, fixed FX controls layout.
at minimal size, with focus buttons:
deere-compact-fx_2

@ronso0
Copy link
Member Author

ronso0 commented Nov 22, 2018

@Be-ing What do ou think about moving the Master FX buttons to the toolbar? And rearranging the mixer knobs a bit?
before:
deere-fx-mixerbar-before

after, #1848 merged:
deere-fx-mixerbar-after

@Be-ing
Copy link
Contributor

Be-ing commented Nov 22, 2018

Nice, I like this. I think it makes sense following our discussion on Zulip about the effects GUI.

What do ou think about moving the Master FX buttons to the toolbar? And rearranging the mixer knobs a bit?

Does it fit with the booth gain knob too? Regardless, it's a bit cluttered up there. It would help if we replaced the buttons with text, which takes up a lot of space, with icons.

@ronso0
Copy link
Member Author

ronso0 commented Nov 23, 2018

Does it fit with the booth gain knob too?

Not really, the center gap is about the same before/after. Labels get squeezed at minimal size..
As you may know I prefer icons over text, let's talk about that after #1848

@ronso0
Copy link
Member Author

ronso0 commented Nov 23, 2018

wookay, this is how it looks now.

Finally, compact units aren't stretching horizontally anymore when the opposing unit is expanded.
And I removed the META label, hope that's okay. It kinda felt obsolete since the label isn't there when the unit's collapsed, and Super/Mix don't have a label as well.

Please test!
deere-compact-fx-after

@Be-ing
Copy link
Contributor

Be-ing commented Nov 23, 2018

Hmm... aesthetically something feels a bit odd about the collapsed view. Maybe try not having the expand button and the FX# label not on the same row?

And I removed the META label, hope that's okay.

Yes, thank you.

@ronso0
Copy link
Member Author

ronso0 commented Nov 24, 2018

Hmm... aesthetically something feels a bit odd about the collapsed view. Maybe try not having the expand button and the FX# label not on the same row?

I see what you mean. The label doesn't fit in between expand adn Pfl button, the unit would need to grow then. Or what do you have in mind?
I could try a border around the label, or group it with the expand toggle in another way:
deere-fxlabel-border

@Be-ing
Copy link
Contributor

Be-ing commented Nov 24, 2018

How does it look with the expand button and headphone button side by side?

@ronso0
Copy link
Member Author

ronso0 commented Nov 24, 2018

How does it look with the expand button and headphone button side by side?

Will be tricky, they're differnetly sized. Also, the FX label is the header so it should stay at the top IMO

@ronso0
Copy link
Member Author

ronso0 commented Nov 24, 2018

The rules I try to follow:

  • FX = header
  • expand toggle sits in the top-right corner

So they could be grouped with a border like pictured above, or the expand toggle is stretched and includes the FX label:
deere-fxlabel-button

@Be-ing
Copy link
Contributor

Be-ing commented Nov 24, 2018

Hm, that looks better, but I'm not certain it's what we should ultimately go with. Perhaps we can merge that and if we come up with better ideas later, open a new PR?

@ronso0
Copy link
Member Author

ronso0 commented Nov 24, 2018

Hm, that looks better, but I'm not certain it's what we should ultimately go with. Perhaps we can merge that and if we come up with better ideas later, open a new PR?

I agree. Will implement that later on.
For consistency the sampler expand toggle goes to the right, as well:
deere-sampler-expand

Only drawback: the Library expand toggle at the left looks kinda lost now. Should we move that to the right side of the search box?

@ronso0
Copy link
Member Author

ronso0 commented Nov 26, 2018

I just force-pushed to clean up the commits a bit:

  • merge FX label expand toggle
  • move sampler expand toggle
  • move library expand toggle right nex to search bar

deere-compactfx-finish

@ronso0
Copy link
Member Author

ronso0 commented Dec 3, 2018

@Be-ing Any objections? Ready to merge?

@Be-ing
Copy link
Contributor

Be-ing commented Dec 26, 2018

Something about the mix knob being adjacent to a rectangular button looks a little odd, but I think this does improve the design. Perhaps if you come up with an idea to improve that you can open a new PR.

@Be-ing Be-ing merged commit db3dfc6 into mixxxdj:master Dec 26, 2018
@ronso0 ronso0 deleted the deere-compact-fx branch December 28, 2018 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants