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

Restore some whitespace to the mixer channel layout #7507

Merged
merged 5 commits into from
Sep 19, 2024

Conversation

sakertooth
Copy link
Contributor

@sakertooth sakertooth commented Sep 18, 2024

Trying to reach a compromise with #7505 and #7502.

I think I initially believed that users were complaining about the spacing being too much horizontally. However, the vertical spacing in QVBoxLayout was 6 for me by default, which could be a bit large, and wasn't set to something smaller in #6591.

I changed the margin size and spacing to make the mixer channel a bit shorter, while also trying to not compromise useful whitespace.

I'm waiting for two approvals for this PR in particular before merge (I should probably require two approvals in general anyways).

Fixes #7505 (I think)

@michaelgregorius
Copy link
Contributor

Look of the mixer with the changes in this PR as of now:

7502-CompromiseWith3

Since the spacing was reduced to 3, this should not be needed anymore
@sakertooth
Copy link
Contributor Author

Thoughts @michaelgregorius?

@sakertooth
Copy link
Contributor Author

Here's an updated image for how the mixer channel's look now:

image

I am very satisfied with this, but I'm waiting for other people's opinions before merge.

Copy link
Contributor

@Rossmaxx Rossmaxx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

@michaelgregorius
Copy link
Contributor

I am very satisfied with this, but I'm waiting for other people's opinions before merge.

If this was mainly about the horizontal space then I'd appreciate if there was a bit more margin at the bottom where the fader ends.

Otherwise it looks okay to me.

@sakertooth
Copy link
Contributor Author

sakertooth commented Sep 19, 2024

What do we think about moving the peak indicator to the bottom? It adds some space for the fader at the bottom, and I think looks better? I probably won't merge this in (I'm foreshadowing backlash), but just a thought.

image

@Rossmaxx
Copy link
Contributor

Rossmaxx commented Sep 19, 2024

Peak indicator at bottom doesn't look good according to me. It seems to increase the view angle, but that might be just me. It's something i cam get used to so it's not really a blocker.

@sakertooth
Copy link
Contributor Author

Peak indicator at bottom doesn't look good according to me. It seems to increase the view angle, but that might be just me. It's something i cam get used to so it's not really a blocker.

Yeah, I'll leave it for now but just add some more whitespace at the bottom for the fader.

@sakertooth
Copy link
Contributor Author

How it looks as of the time of merge:
image

@sakertooth sakertooth merged commit c952d56 into LMMS:master Sep 19, 2024
11 checks passed
@sakertooth sakertooth deleted the restore-mixer-channel-padding branch September 19, 2024 16:00
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.

Mixer looks cramped and has decreased usability
3 participants