Allow an About menu item in File to be used as special #4934
Conversation
… macOS about in the app menu
|
I suppose this is targeted towards 2.6 given that we are in a feature freeze, right? |
|
Hmm, in a sense yes. But this is a small enabler that should have landed months ago as part of the new About window in fyne-x. Given that I'm tempted to try and convince you there is no new feature here (and certainly no new api :) ) |
|
Sending my apologies as this was discovered in git stashes very late in the release process and I should have processed them all ages ago. |
|
Tested it and it works, I can't say I'm a fan of the "magic strings" approach but it's what was already in place for settings/preferences. We should document this better because I didn't even know this existed. Also we'll need to think about the magic string approach now that we have localization/translation, but that doesn't block this PR. |
internal/driver/glfw/menu_darwin.m
Outdated
| item = [menu itemArray][0]; | ||
| [item setAction:@selector(tapped:)]; | ||
| [item setTarget:[FyneMenuHandler class]]; | ||
| [item setTag:id+menuTagMin]; |
There was a problem hiding this comment.
id is a keyword. What does it do here?
|
I'm sorry; I like the idea behind the change but I'm not that keen on letting things through, it defeats the purpose of having a feature freeze. Given that idea with it is to let things stabilise and allow us to focus more on bug fixing, I'd be tempted to say that merging PR:s that already are open is a step too far already. What's the point in having a process for the feature freeze if we don't follow it? |
Essentially the purpose of a release schedule on a large open source project is about corralling energy towards the release. If we cut off at a specific time all work because a date has passed then the enthusiasm that was going into those items wanes and we have to work again to resuscitate the item 3-4 months later after we have been working on bug fixing. I don't wish to break process but I do want to see the best release we can make in the approximate timescales. |
|
In this specific instance it is absolutely my fault - the about screen landed in fyne-x about 3 months ago and I forgot about this tweak that makes it more useful. The result will be macOS apps with multiple About menu entries for the next 4-6 months... |
internal/driver/glfw/menu_darwin.go
Outdated
| if i < len(menu.Items)-1 { | ||
| items = append(items, menu.Items[i+1:]...) | ||
| } |
There was a problem hiding this comment.
This condition is not necessary. I vote to remove it again.
There was a problem hiding this comment.
Also, the two cases of the switch contain the same code, AFAICS. That should be consolidated.
There was a problem hiding this comment.
And: Where are the tests?
Please add an “About” case to the existing test in menu_darwin_test.go.
Also, I’d really hoped for a test checking the flaws that were introduced without being detected.
There was a problem hiding this comment.
I reverted the code to the older recursive version - some of the test cases seemed to only work that way. About is now added to tests
to replace the macOS about in the app menu
Checklist: