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

LateNight Skin, moving Mixxx Logo #4677

Merged
merged 4 commits into from
Feb 22, 2022

Conversation

Russe
Copy link
Contributor

@Russe Russe commented Feb 12, 2022

Moving ToolbarLogo that there's less distraction between clock and CPU Load widgets.

Moving ToolbarLogo that there's less distraction between clock and CPU Load widgets.
@github-actions github-actions bot added the skins label Feb 12, 2022
@Russe Russe changed the title Update toolbar.xml LateNight Skin, moving Mixxx Logo Feb 12, 2022
@Swiftb0y
Copy link
Member

Would you mind posting a picture before and after your changes?

@ronso0
Copy link
Member

ronso0 commented Feb 12, 2022

In the new place the logo is distracting IMO.
I'd go along with moving the logo to the far left, though.

FWIW with multi-core CPUs the CPU Load bar is not a reliable indicator for buffer over/underruns anymore.
I suggest you rather play as usual (pitch change, fx, 3 or 4 decks and maybe samplers playing) and watch the underrun counter at the bottom of Preferences > Sound Hardware. If it's 0 (or close to zero and you don't hear any annoying audio glitches) you can lower the latency until you do see the counter increase. Then go back the previous value.

@ronso0
Copy link
Member

ronso0 commented Feb 12, 2022

@Russe
Copy link
Contributor Author

Russe commented Feb 12, 2022

Would you mind posting a picture before and after your changes?

latenight-original
latenight-change

@Russe
Copy link
Contributor Author

Russe commented Feb 12, 2022

In the new place the logo is distracting IMO. I'd go along with moving the logo to the far left, though.

Yes, I will add a screenshot first.

@Russe
Copy link
Contributor Author

Russe commented Feb 12, 2022

Thanks for information about latency.

@Russe
Copy link
Contributor Author

Russe commented Feb 12, 2022

Mixxx logo on the left side

latenight-change2

@ronso0
Copy link
Member

ronso0 commented Feb 12, 2022

Okay, I think this looks nice if there is more space at the left of the logo, like in between MAX LIBRARY and WAVEFORMS.

@Russe
Copy link
Contributor Author

Russe commented Feb 12, 2022

PaleMoon and Classic is a bit different, I used an empty WidgetGroup, and not the ToolbarSeparator with the dots at Classic Skin.

latenight-change3-palemoon
latenight-change3-classic

@JoergAtGithub
Copy link
Member

IMHO the logo would look best where SETTINGS is now, maybe move settings to the other widget toggle buttons on the left?

@ronso0
Copy link
Member

ronso0 commented Feb 12, 2022

IMHO the logo would look best where SETTINGS is now, maybe move settings to the other widget toggle buttons on the left?

That would require moving the cursor over half the screen to get to the menu. The point is to have the settings toggle close to the menu.

@Russe
Copy link
Contributor Author

Russe commented Feb 13, 2022

Here's a proposal with Mixxx logo in the right side, without moving the settings button.
I agree, the settings button should stay on the right side.

latenight-change5-palemoon
latenight-change5-classic

This version is my favourite.

@ywwg
Copy link
Member

ywwg commented Feb 13, 2022

what about centered? :)

I do like the right-justified version too

@Russe
Copy link
Contributor Author

Russe commented Feb 13, 2022

I don't like the centered, that was my first attempt. Problem is, that it's centered within the white space, but not centered within the window with. So the logo will be displayed a bit to the right side of the window title.

I will update the code like in the last screenshot, so the logo will be on the right side.

Moving Toolbar Logo that there's less distraction between clock and CPU Load widgets.
@ronso0
Copy link
Member

ronso0 commented Feb 16, 2022

I don't like the centered, that was my first attempt. Problem is, that it's centered within the white space,

Even though I think this is possible, I think we should not put the eye-catching logo in the center. It's distracting from all that already happens in the center column, mixer, pitch fader etc.

I do like the current proposal!
Please squash the commits, name it "LateNight: move logo to the right" and rebase onto mixxx/main.

@Russe
Copy link
Contributor Author

Russe commented Feb 16, 2022

Sorry, I got lost, this is my first contribution.
Could you explain where I can squash my commits? I'm working on github.com, and not with a git client.

I found something about in the docs:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/merging-a-pull-request
But I can not see the merge or squash button.

@ronso0
Copy link
Member

ronso0 commented Feb 16, 2022

I'm working on github.com, and not with a git client.

Okay, never mind. We will do that when merging. (only mixxxdj members see the merge options)

Since this is your first contribution we need you to sign the contributor agreement and thereby allow Mixxx to include your changes.
Thank you!

@ronso0
Copy link
Member

ronso0 commented Feb 16, 2022

Oh, and we need to update the preview screen for the preferences.
@ywwg when you approve the current state I can do that (I'd simply push pixels in Gimp)

@Russe
Copy link
Contributor Author

Russe commented Feb 17, 2022

OK, thanks.
I already signed the agreement, because I created the drawing of Traktor Kontrol Z2 controller and JoergAtGithub asked me to sign it.

If you want me I can update the screenshots/previews as well within this pull request, so please advise.

@ronso0
Copy link
Member

ronso0 commented Feb 17, 2022

Sure you can change the screenshots, too. In order to avoid creating a screenshot from scratch, just swap the buttons and the logo with an image editor so the look matches this PR, and try to keep the image quality (size) of the original if possible.
Note this needs to be done for both skin_preview_Classic.png and skin_preview_PaleMoon.png
Thank you!

@ronso0
Copy link
Member

ronso0 commented Feb 17, 2022

@ywwg When you give your final 👍please squash & merge, rename to "LateNight: move logo to the right"
Thanks!

@Russe
Copy link
Contributor Author

Russe commented Feb 21, 2022

Just tested the installation on Ubuntu with the build deb-file from here. Everything looks as expected.
Is there anything I need to to do for this PR?
I don't want to rush you, just asking if something is missing from my side.

@ronso0
Copy link
Member

ronso0 commented Feb 21, 2022

No, nothing left to do, except waiting for the Okay! of another team member. Then we can squash and merge.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM Thank you.

@daschuer daschuer merged commit f3a2ff2 into mixxxdj:main Feb 22, 2022
@Russe Russe deleted the main-LateNightSkin-adaption branch February 22, 2022 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants