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

fixed nsmenu activation bug #1918

Closed
wants to merge 2 commits into from

Conversation

adamnemecek
Copy link

@adamnemecek adamnemecek commented Apr 28, 2021

I'm looking to get feedback on this. I'm not sure if the commented out is necessary, but the menu seems to work fine.

@adamnemecek adamnemecek changed the title fixed activation bug fixed nsmenu activation bug Apr 28, 2021
@adamnemecek
Copy link
Author

adamnemecek commented Apr 28, 2021

@ArturKovacs @madsmtm @casperstorm do you guys want to verify this? i saw @madsmtm's fix, but it doesn't quite fix it.

@maroider
Copy link
Member

So this is meant as an alternative to #1903, right?

@adamnemecek
Copy link
Author

#1903 doesn’t work for me. Does it for you?

@maroider
Copy link
Member

maroider commented Apr 28, 2021

I don't have a mac, so I can't really test either PR. I've just got this repo set to "watch all activity", like some kind of lunatic.

@ArturKovacs
Copy link
Contributor

Awesome work! Do you mind me pushing to this branch?

@adamnemecek
Copy link
Author

I don’t mind but are you sure we don’t care about the code I uncommented? Are you on discord? I’d like to discuss some other nsmenu matters.

@ArturKovacs
Copy link
Contributor

I'm ArturBarnabas#8576 on discord, I wrote to someone, who I believe is you @adamnemecek

@lemarier
Copy link

lemarier commented Apr 28, 2021

GUYS!!!! IT WORKS 💯

At @tauri-apps we are blocked by this !!!!

Good job!!! ;)

@ArturKovacs
Copy link
Contributor

@adamnemecek please check if my changes look good.

@ArturKovacs ArturKovacs marked this pull request as ready for review April 28, 2021 21:38
@lemarier
Copy link

Well done! Works fine here. Tested with multi-window as well!

@ArturKovacs
Copy link
Contributor

@adamnemecek you seem to be pretty competent at this stuff are you interested in becoming a winit maintainer? @francesca64

@adamnemecek
Copy link
Author

adamnemecek commented Apr 28, 2021

Sure. It’s a thankless job but someone has to do it. I’ll look at your changes in a bit.

@ArturKovacs
Copy link
Contributor

Tested with multi-window as well!

Oh no! I just tested that as well and now only the first window shows up. All windows show up on the master branch. Will have to spend some more time with this...

@lemarier
Copy link

Tested with multi-window as well!

Oh no! I just tested that as well and now only the first window shows up. All windows show up on the master branch. Will have to spend some more time with this...

Here's the 3 windows are created and show's, I wonder if it's not because they are hidden behind another window?
Running 11.2.3

@madsmtm
Copy link
Member

madsmtm commented Apr 28, 2021

Honestly, at this point I'm beginning to think the examples are the fault; things work as they should if you create the windows after applicationDidFinishLaunching:

Event::NewEvents(event) if event == StartCause::Init => {
    for _ in 0..3 {
        let window = Window::new(&event_loop).unwrap();
        windows.insert(window.id(), window);
    }
}

Maybe it's just not possible to create windows in a clean way, where they show up from the beginning, without resorting to weird hacks?

@madsmtm
Copy link
Member

madsmtm commented Apr 28, 2021

Also, can you please state your MacOS versions? Because there's no difference between #1903 and this on my machine (MacOS 10.14.6 Mojave), but as I said in the linked issue, we would probably have to do something similar to linebender/druid#994 to support higher versions - but it would be nice to know if it is indeed a version-dependent thing!

@adamnemecek
Copy link
Author

I’m on Big Sur. This fix is in fact the same thing as what Druid is doing.

@ArturKovacs
Copy link
Contributor

I'm on macOS 10.11. And just to clarify I'm affected by both the menubar not activating and windows not showing up.

I wonder if it's not because they are hidden behind another window?

I start up the example then drag and move the first window away from its initial position and there's no window behind it. Then I close it and a new window suddenly appears at the original spot.

@casperstorm
Copy link
Contributor

I'm on macOS 10.11. And just to clarify I'm affected by both the menubar not activating and windows not showing up.

I wonder if it's not because they are hidden behind another window?

I start up the example then drag and move the first window away from its initial position and there's no window behind it. Then I close it and a new window suddenly appears at the original spot.

Just to tip in: I'm on 10.15 and it is working for me, both single and multiwindow.

@adamnemecek adamnemecek mentioned this pull request Apr 29, 2021
8 tasks
@ArturKovacs
Copy link
Contributor

Closing as this was superseded by #1903 and #1922

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants