Fix child touch bar items not updating by MarshallOfSound · Pull Request #11812 · electron/electron · GitHub
Skip to content

Fix child touch bar items not updating#11812

Merged
codebytere merged 2 commits into
masterfrom
fix-child-touch-bar-items
Feb 12, 2018
Merged

Fix child touch bar items not updating#11812
codebytere merged 2 commits into
masterfrom
fix-child-touch-bar-items

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

Deep children of a TouchBar didn't cause the top level touch bar to update, now they do

Fixes #11761.

@MarshallOfSound MarshallOfSound requested a review from a team February 2, 2018 11:34
Deep children of a TouchBar didn't cause the top level touch bar to update, now they do

Fixes #11761.
@MarshallOfSound MarshallOfSound force-pushed the fix-child-touch-bar-items branch from 062588d to b55a770 Compare February 2, 2018 11:36
@MarshallOfSound

Copy link
Copy Markdown
Member Author

@ckerr ckerr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code basically LGTM.

Any chance of getting tests for this, or is that too hard to set up?

Comment thread lib/browser/api/touch-bar.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newValue is unused.

@codebytere codebytere left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per @MarshallOfSound there doesn't really seem to be better way to do tests for this, so 👍

@codebytere codebytere merged commit 66b5785 into master Feb 12, 2018
@codebytere codebytere deleted the fix-child-touch-bar-items branch February 12, 2018 17:53
sethlu pushed a commit to sethlu/electron that referenced this pull request May 3, 2018
* Fix child touch bar items not updating

Deep children of a TouchBar didn't cause the top level touch bar to update, now they do

Fixes electron#11761.

* Remove unused newValue property in TB setter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants