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

fix: make TouchBarPopover and TouchBarGroup work #27901

Merged
merged 3 commits into from Mar 1, 2021

Conversation

erickzhao
Copy link
Member

@erickzhao erickzhao commented Feb 24, 2021

Description of Change

Fixes #26615

257cca0: The ordereredItems property was renamed to orderedItems, but this typo was never fixed at the Objective-C layer, leading to early exits when creating TouchBarGroup and TouchBarPopover items.

5b8bd57: As pointed out in #26615 (comment) and #26615 (comment), items inside a TouchBarGroup or TouchBarPopover would add themselves as a parent.

43e108b: Before this change, when adding a new @ImmutableProperty or @LiveProperty, its constructor hook would be called before the existing hooks before it. This caused a bug when instantiating Popovers and Groups -- even though we set the id property first, it would be undefined when we used its value in the hook for the child property a few lines later.

Tester code (adapted from #9025): https://gist.github.com/10e5c142126b439282b97cd762a24d55

cc @MarshallOfSound

Checklist

Release Notes

Notes: Fixed bug where TouchBarPopover and TouchBarGroup were no longer rendering

@electron-cation electron-cation bot added the new-pr 🌱 PR opened in the last 24 hours label Feb 24, 2021
@erickzhao erickzhao added the semver/patch backwards-compatible bug fixes label Feb 25, 2021
@electron-cation electron-cation bot removed the new-pr 🌱 PR opened in the last 24 hours label Feb 25, 2021
@erickzhao erickzhao marked this pull request as ready for review February 26, 2021 23:04
@MarshallOfSound MarshallOfSound merged commit 2d0ad0b into electron:master Mar 1, 2021
@release-clerk
Copy link

release-clerk bot commented Mar 1, 2021

Release Notes Persisted

Fixed bug where TouchBarPopover and TouchBarGroup were no longer rendering

@erickzhao erickzhao deleted the erick/touchbar branch March 1, 2021 21:47
@ggreco
Copy link

ggreco commented Mar 25, 2021

This has been merged 4 weeks ago, I thought that the yesterday v12 release could include it, but it's still broken, any ETA for the backport of the PR to v11 and v12 that are affected by the problem?

@erickzhao
Copy link
Member Author

@ggreco I forgot to add the proper backport labels for this process to be automated. I'll have to apply these backports manually. Thanks for checking in!

@ggreco
Copy link

ggreco commented Mar 26, 2021

@ggreco I forgot to add the proper backport labels for this process to be automated. I'll have to apply these backports manually. Thanks for checking in!

If the labels are the one I see in the bug #26615 it still misses the 12-x-y one.

erickzhao added a commit to erickzhao/ericktron that referenced this pull request Mar 26, 2021
* fix: use correct `orderedItem` touchbar property

* fix: correct parent in touchbar group and popover

* fix: preserve property hook order
@trop
Copy link
Contributor

trop bot commented Mar 26, 2021

@erickzhao has manually backported this PR to "12-x-y", please check out #28411

erickzhao added a commit to erickzhao/ericktron that referenced this pull request Mar 26, 2021
* fix: use correct `orderedItem` touchbar property

* fix: correct parent in touchbar group and popover

* fix: preserve property hook order
@trop
Copy link
Contributor

trop bot commented Mar 26, 2021

@erickzhao has manually backported this PR to "11-x-y", please check out #28412

codebytere pushed a commit that referenced this pull request Mar 29, 2021
* fix: use correct `orderedItem` touchbar property

* fix: correct parent in touchbar group and popover

* fix: preserve property hook order
codebytere pushed a commit that referenced this pull request Mar 29, 2021
* fix: use correct `orderedItem` touchbar property

* fix: correct parent in touchbar group and popover

* fix: preserve property hook order
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/touch-bar semver/patch backwards-compatible bug fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TouchBar: Groups and popover-Items not rendering items at all
4 participants