Require setting the activation policy on the event loop#1922
Require setting the activation policy on the event loop#1922ArturKovacs merged 5 commits intorust-windowing:masterfrom
Conversation
|
@madsmtm please review this. |
madsmtm
left a comment
madsmtm
left a comment
There was a problem hiding this comment.
Gladly!
Aside: Sorry for the delay on #1759, I will take a look at it some point, just haven't mustered the energy to do it yet - and probably don't have the time to commit myself as a maintainer yet, but again, thanks for the suggestion, I'll let you know if I feel I can take on that responsibility.
| pub struct AuxDelegateState { | ||
| /// We store this value in order to be able defer setting the activation policy until | ||
| /// after the app has finished launching. If the activation policy is set earlier, the | ||
| /// menubar is unresponsive for example. |
There was a problem hiding this comment.
Add note about Catalina, e.g.:
| /// menubar is unresponsive for example. | |
| /// menubar, for example, is initially unresponsive on Catalina and above. |
There was a problem hiding this comment.
This is not true, it's also unresponsive on my 10.11. Is there a version where it's responsive from the start? If there is this could be worded like "the menubar is unresponsive on some systems"
There was a problem hiding this comment.
On the current master, that is, after #1903, the menu is responsive on my machine, running MacOS 10.14.6 Mojave - and I thought it was on yours as well? But if I'm mistaken there, then you're right, the menubar is unresponsive on some systems is better.
There was a problem hiding this comment.
Oh. Now I understand what I was confused about. Yes, after #1903 everything works on my system (10.11), but now you enlightened me that that's not the case on 10.15 and 11.
| /// Safety: Assumes that Object is an instance of APP_DELEGATE_CLASS | ||
| pub unsafe fn get_aux_state_mut(this: &Object) -> RefMut<'_, AuxDelegateState> { | ||
| let ptr: *mut c_void = *this.get_ivar(AUX_DELEGATE_STATE_NAME); | ||
| // Watch out that this needs to be the correct type |
There was a problem hiding this comment.
Can't you just set ptr to the correct type above? (If not, there's probably something in Rust's type conversion that I don't understand, bear with me)
There was a problem hiding this comment.
The ivar itself is *mut c_void so I guess this better exposes what is happening.
But as far as I know, I could just write let ptr: *mut RefCell<AuxDelegateState> = ... but I'm not a 100% sure and I'm also not sure if the ivar has to be a void pointer instead of a pointer with the actual type.
There was a problem hiding this comment.
Fair enough, let's keep it as-is, this we at least know is correct
madsmtm
left a comment
madsmtm
left a comment
There was a problem hiding this comment.
Your changes looks fine, now it's just comments 😉
Co-authored-by: Mads Marquart <[email protected]>
|
This created a regression: #2051 |
3 participants
The activation policy could previously be specified through the window builder. This was incorrect because the activation policy is an application-wide property, not window-specific.
cargo fmthas been run on this branchcargo docbuilds successfullyCHANGELOG.mdif knowledge of this change could be valuable to users