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

MixxxMainWindow: Hide menubar during startup #3839

Merged
merged 2 commits into from
May 9, 2021

Conversation

Holzhaus
Copy link
Member

@Holzhaus Holzhaus commented May 7, 2021

Right now, the style of the menu bar changes during startup. This
doesn't look nice. This commit simply hides the menubar during that
period.

Right now, the style of the menu bar changes during startup. This
doesn't look nice. This commit simply hides the menubar during that
period.
@Holzhaus Holzhaus added the ui label May 7, 2021
@Holzhaus Holzhaus added this to the 2.3.0 milestone May 7, 2021
@Holzhaus Holzhaus requested a review from ronso0 May 7, 2021 09:01
@ronso0
Copy link
Member

ronso0 commented May 7, 2021

why wait so long, until after rescanning the library and optionally loading tracks passed by args?
can't we show the menbar directly after emit(skinLoaded); ?

When I was trying to get Mixxx into a state where the GUI is not available and the menubar may be needed (Shade configured, skin.xml broken), I discovered an issue with loadDefaultSkin: Mixxx wouldn't look for or wouldn't find LateNight, and show a warning > Okay > close.

apart from that it works fine and I can't see what could go wrong.

related:
Another cosmetic issue is that we're entering fullscreen very late, the window decoration is visible before the skin finished loading. That can also happen earlier IMO, after pConfig is read and its permssion are set.
That way we'd have a clean, fullscreen launch screen, then the skin is shown with the styled menubar.

@daschuer
Copy link
Member

daschuer commented May 7, 2021

How much work would it be to style the menu bar in the startup screen? Is it just a matter of copy and paste?
If yes I would prefer this to avoid last minute issues. Messing with the menu bar is .. just a mess .. due to native OS code.

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

Looks like we'd either have to split out the menubar styles and parse them separately like the launch screen. Or do some other magic with the stylesheet of the configured skin and extract the menubar styles.
Too much work for too little benefit IMO. Also, as I mentioned earlier, at least for me the menubar is not responsive until the skin has finished loading and specific menu hotkeys also don't work (Alt+H for Help for example) -- they're ether dropped or issued only after skin load.

This solution here is perfect IMO, we have a clean launch screen now.
But we should fixx the 'default skin' load issue mentioned above otherwise users can not switch back from a broken (custom?) skin to an official skin from the GUI, only by editing mixxx.cfg

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

7999e17

9110a07

both work fine here.

@Holzhaus
Copy link
Member Author

Holzhaus commented May 7, 2021

@ronso0 Added the first commit (don't know if the second one is related to this PR). Ready to merge?

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

Looking good. All keyboard shortcuts still work (IIRC had issues with this in related PRs)
@daschuer Can you confirm those work with Gnome?

I wanted to give this a shot on Windows10 but all the Zip artifacts containing the msi are currently broken..

Also, I'm curious what happens on macOS (if anything..)
@foss- Could you please test this?

@Holzhaus
Copy link
Member Author

Holzhaus commented May 7, 2021

@ronso0 what do you mean with the zip artifacts are broken? Is the msi file invalid? Can't the zip files be extracted?

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

Added the first commit (don't know if the second one is related to this PR)

aah, you caught me... Will open a separate PR for this.

@JoergAtGithub
Copy link
Member

I just tried teh MSI artifact from this PR and it works as intended on Win7 !

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

@ronso0 what do you mean with the zip artifacts are broken? Is the msi file invalid? Can't the zip files be extracted?

the contained msi is 0 bytes. tried with artifacts from multiple PRs, all the same.

ronso@ropad580:~/Desktop$ unzip mixxx_hide-menubar.zip 
Archive:  mixxx_hide-menubar.zip
  End-of-central-directory signature not found.  Either this file is not
  a zipfile, or it constitutes one disk of a multi-part archive.  In the
  latter case the central directory and zipfile comment will be found on
  the last disk(s) of this archive.
unzip:  cannot find zipfile directory in one of mixxx_hide-menubar.zip or
        mixxx_hide-menubar.zip.zip, and cannot find mixxx_hide-menubar.zip.ZIP, period.

ronso@ropad580:~/Desktop$ zip -FF mixxx_hide-menubar.zip -O mixxx_hide-menubar.fixed.zip
Fix archive (-FF) - salvage what can
	zip warning: Missing end (EOCDR) signature - either this archive
                     is not readable or the end is damaged
Is this a single-disk archive?  (y/n): y
  Assuming single-disk archive
Scanning for entries...
 copying: mixxx-2.3-beta-3898-g7670b8ad01.msi 
	zip warning: no end of stream entry found: mixxx-2.3-beta-3898-g7670b8ad01.msi
	zip warning: rewinding and scanning for later entries
	zip warning: zip file empty

ronso@ropad580:~/Desktop$ ls -l mixxx_hide-menubar.zip 
-rw-rw-r-- 1 ronso ronso 59834360 Mai  7 21:54 mixxx_hide-menubar.zip

@Holzhaus
Copy link
Member Author

Holzhaus commented May 7, 2021

@ronso0 I cannot reproduce:

$ unzip Windows\ Installer.zip
Archive:  Windows Installer.zip
  inflating: mixxx-2.3-beta-3896-g57bb23f94f.msi
$ ls -lh mixxx-2.3-beta-3896-g57bb23f94f.msi
Permissions Size User Group Date Modified Name
.rw-r--r--   73M jan  jan    7 May 17:14  mixxx-2.3-beta-3896-g57bb23f94f.msi

And if the ZIP was broken, there's nothing we can do about it, because it's zipped automatically by GitHub, not our CI.

@JoergAtGithub
Copy link
Member

grafik
grafik

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

now it works. looks like I got broken dwonloads repeatedly, dunno why.
if there were checksums for artifacts I'd probably have noticed earlier.

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

@JoergAtGithub
All keyboard shortcuts from the menus do work?

@JoergAtGithub
Copy link
Member

JoergAtGithub commented May 7, 2021

I just checked this and I found out, that the keyboard shortcuts of the View menu only work, if the menu is not shown. For the other menus this works.

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

the keyboard shortcuts of the View menu only work, if the menu is not shown

you mean: have the menu open AND press any of the shown key combos? why? : )
does that work in the current 2.3 build?

@Holzhaus
Copy link
Member Author

Holzhaus commented May 7, 2021

the keyboard shortcuts of the View menu only work, if the menu is not shown

If the menu is not shown? Weird.

@ronso0
Copy link
Member

ronso0 commented May 7, 2021

Can confirm, only F11 works with the menu open. same in 2.3
so it s unrelated, a bug in https://github.com/mixxxdj/mixxx/blob/2.3/src/widget/wmainmenubar.cpp#L711

@Holzhaus
Copy link
Member Author

Holzhaus commented May 7, 2021

OK, so this PR doesn't need further changes?

@ronso0 ronso0 mentioned this pull request May 8, 2021
@ronso0
Copy link
Member

ronso0 commented May 8, 2021

I'm hesitant. Issues were discovered in earlier menubar PRs (admittedly they were more complex) so I'd rather wait for feedback from other distros, like Ubuntu with the global menubar and macOS.
@daschuer
@foss-

@Holzhaus
Copy link
Member Author

Holzhaus commented May 8, 2021

Ok

@foss-
Copy link
Contributor

foss- commented May 8, 2021

Tested Artifact Build and did not run into problems.

@ronso0
Copy link
Member

ronso0 commented May 8, 2021

so all buttons and shortcuts work?

I'll test the global menu with xfce and gnome tonight.

@foss-
Copy link
Contributor

foss- commented May 8, 2021

Not sure about all but I tried shortcuts from menubar and a few I frequently use as well as the buttons in UI.

One odd behavior I noticed is that a single click on increasing beatloop size triggers multiple steps. so while on 4 I suddenly have 256 beats :/ Probably unrelated to this PR and not 100% reproducible. Rather related to known laggyness issues with mixxx ui on macOS leading to all sorts of strange behavior while being hard to reproduce and inconsistent. Currently in a state. Right now had a situation where clicks where not registered (or resulting in instant responce) and after a few seconds were all executed at once resulting in a 1/16 loop. Nice if you are playing an experimental set. Not nice if you want to keep a more mainstream set flowing.

@ronso0
Copy link
Member

ronso0 commented May 8, 2021

Thanks, I was referrign to menubar buttons and their keyboard shortcuts. For the other unrelated issues, please file bug report(s) so we remember them.

@daschuer
Copy link
Member

daschuer commented May 8, 2021

I can confirm this is working with Ubuntu Unity and Gnome on Focal and Bionic.

@ronso0
Copy link
Member

ronso0 commented May 9, 2021

perfecto! Thanks @Holzhaus

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.

5 participants