Fix child touch bar items not updating (#11812)

* 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 #11761.

* Remove unused newValue property in TB setter

atom/browser/ui/cocoa/atom_touch_bar.mm

@@ -135,6 +135,8 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
     [self updateSegmentedControl:(NSCustomTouchBarItem*)item withSettings:settings];
   } else if (item_type == "scrubber") {
     [self updateScrubber:(NSCustomTouchBarItem*)item withSettings:settings];
+  } else if (item_type == "group") {
+    [self updateGroup:(NSGroupTouchBarItem*)item withSettings:settings];
   }
 }
 
@@ -169,18 +171,31 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
   auto identifier = [self identifierFromID:item_id type:item_type];
   if (!identifier) return;
 
-  std::vector<std::string> popover_ids;
+  std::vector<mate::Dictionary> parents;
-  settings.Get("_popover", &popover_ids);
+  settings.Get("_parents", &parents);
-  for (auto& popover_id : popover_ids) {
+  for (auto& parent : parents) {
-    auto popoverIdentifier = [self identifierFromID:popover_id type:"popover"];
+    std::string parent_type;
-    if (!popoverIdentifier) continue;
+    std::string parent_id;
+    if (!parent.Get("type", &parent_type) || !parent.Get("id", &parent_id))
+      continue;
+    auto parentIdentifier = [self identifierFromID:parent_id type:parent_type];
+    if (!parentIdentifier) continue;
 
-    NSPopoverTouchBarItem* popoverItem =
+    if (parent_type == "popover") {
-        [touchBar itemForIdentifier:popoverIdentifier];
+      NSPopoverTouchBarItem* popoverItem =
-    [self refreshTouchBarItem:popoverItem.popoverTouchBar
+        [touchBar itemForIdentifier:parentIdentifier];
-                           id:identifier
+      [self refreshTouchBarItem:popoverItem.popoverTouchBar
-                     withType:item_type
+                            id:identifier
-                 withSettings:settings];
+                      withType:item_type
+                  withSettings:settings];
+    } else if (parent_type == "group") {
+      NSGroupTouchBarItem* groupItem =
+        [touchBar itemForIdentifier:parentIdentifier];
+      [self refreshTouchBarItem:groupItem.groupTouchBar
+                             id:identifier
+                       withType:item_type
+                   withSettings:settings];
+    }
   }
 
   [self refreshTouchBarItem:touchBar
@@ -478,6 +493,17 @@ static NSString* const ImageScrubberItemIdentifier = @"scrubber.image.item";
                                                                       items:generatedItems];
 }
 
+- (void)updateGroup:(NSGroupTouchBarItem*)item
+    withSettings:(const mate::PersistentDictionary&)settings {
+  
+  mate::PersistentDictionary child;
+  if (!settings.Get("child", &child)) return;
+  std::vector<mate::PersistentDictionary> items;
+  if (!child.Get("ordereredItems", &items)) return;
+
+  item.groupTouchBar = [self touchBarFromItemIdentifiers:[self identifiersFromSettings:items]];
+}
+
 - (NSTouchBarItem*)makeSegmentedControlForID:(NSString*)id
                               withIdentifier:(NSString*)identifier {
   std::string s_id([id UTF8String]);

lib/browser/api/touch-bar.js

@@ -135,7 +135,21 @@ class TouchBar extends EventEmitter {
 class TouchBarItem extends EventEmitter {
   constructor () {
     super()
-    this.id = `${nextItemID++}`
+    this._addImmutableProperty('id', `${nextItemID++}`)
+    this._parents = []
+  }
+
+  _addImmutableProperty (name, value) {
+    Object.defineProperty(this, name, {
+      get: function () {
+        return value
+      },
+      set: function () {
+        throw new Error(`Cannot override property ${name}`)
+      },
+      enumerable: true,
+      configurable: false
+    })
   }
 
   _addLiveProperty (name, initialValue) {
@@ -152,22 +166,32 @@ class TouchBarItem extends EventEmitter {
       enumerable: true
     })
   }
+
+  _addParent (item) {
+    const existing = this._parents.some(test => test.id === item.id)
+    if (!existing) {
+      this._parents.push({
+        id: item.id,
+        type: item.type
+      })
+    }
+  }
 }
 
 TouchBar.TouchBarButton = class TouchBarButton extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'button'
+    this._addImmutableProperty('type', 'button')
     const {click, icon, iconPosition, label, backgroundColor} = config
     this._addLiveProperty('label', label)
     this._addLiveProperty('backgroundColor', backgroundColor)
     this._addLiveProperty('icon', icon)
     this._addLiveProperty('iconPosition', iconPosition)
     if (typeof click === 'function') {
-      this.onInteraction = () => {
+      this._addImmutableProperty('onInteraction', () => {
         config.click()
-      }
+      })
     }
   }
 }
@@ -176,16 +200,16 @@ TouchBar.TouchBarColorPicker = class TouchBarColorPicker extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'colorpicker'
+    this._addImmutableProperty('type', 'colorpicker')
     const {availableColors, change, selectedColor} = config
     this._addLiveProperty('availableColors', availableColors)
     this._addLiveProperty('selectedColor', selectedColor)
 
     if (typeof change === 'function') {
-      this.onInteraction = (details) => {
+      this._addImmutableProperty('onInteraction', (details) => {
         this._selectedColor = details.color
         change(details.color)
-      }
+      })
     }
   }
 }
@@ -194,11 +218,10 @@ TouchBar.TouchBarGroup = class TouchBarGroup extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'group'
+    this._addImmutableProperty('type', 'group')
-    this.child = config.items
+    const defaultChild = (config.items instanceof TouchBar) ? config.items : new TouchBar(config.items)
-    if (!(this.child instanceof TouchBar)) {
+    this._addLiveProperty('child', defaultChild)
-      this.child = new TouchBar(this.child)
+    this.child.ordereredItems.forEach((item) => item._addParent(this))
-    }
   }
 }
 
@@ -206,7 +229,7 @@ TouchBar.TouchBarLabel = class TouchBarLabel extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'label'
+    this._addImmutableProperty('type', 'label')
     this._addLiveProperty('label', config.label)
     this._addLiveProperty('textColor', config.textColor)
   }
@@ -216,18 +239,13 @@ TouchBar.TouchBarPopover = class TouchBarPopover extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'popover'
+    this._addImmutableProperty('type', 'popover')
     this._addLiveProperty('label', config.label)
     this._addLiveProperty('icon', config.icon)
-    this.showCloseButton = config.showCloseButton
+    this._addLiveProperty('showCloseButton', config.showCloseButton)
-    this.child = config.items
+    const defaultChild = (config.items instanceof TouchBar) ? config.items : new TouchBar(config.items)
-    if (!(this.child instanceof TouchBar)) {
+    this._addLiveProperty('child', defaultChild)
-      this.child = new TouchBar(this.child)
+    this.child.ordereredItems.forEach((item) => item._addParent(this))
-    }
-    this.child.ordereredItems.forEach((item) => {
-      item._popover = item._popover || []
-      if (!item._popover.includes(this.id)) item._popover.push(this.id)
-    })
   }
 }
 
@@ -235,7 +253,7 @@ TouchBar.TouchBarSlider = class TouchBarSlider extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'slider'
+    this._addImmutableProperty('type', 'slider')
     const {change, label, minValue, maxValue, value} = config
     this._addLiveProperty('label', label)
     this._addLiveProperty('minValue', minValue)
@@ -243,10 +261,10 @@ TouchBar.TouchBarSlider = class TouchBarSlider extends TouchBarItem {
     this._addLiveProperty('value', value)
 
     if (typeof change === 'function') {
-      this.onInteraction = (details) => {
+      this._addImmutableProperty('onInteraction', (details) => {
         this._value = details.value
         change(details.value)
-      }
+      })
     }
   }
 }
@@ -255,8 +273,8 @@ TouchBar.TouchBarSpacer = class TouchBarSpacer extends TouchBarItem {
   constructor (config) {
     super()
     if (config == null) config = {}
-    this.type = 'spacer'
+    this._addImmutableProperty('type', 'spacer')
-    this.size = config.size
+    this._addImmutableProperty('size', config.size)
   }
 }
 
@@ -265,17 +283,17 @@ TouchBar.TouchBarSegmentedControl = class TouchBarSegmentedControl extends Touch
     super()
     if (config == null) config = {}
     const {segmentStyle, segments, selectedIndex, change, mode} = config
-    this.type = 'segmented_control'
+    this._addImmutableProperty('type', 'segmented_control')
     this._addLiveProperty('segmentStyle', segmentStyle)
     this._addLiveProperty('segments', segments || [])
     this._addLiveProperty('selectedIndex', selectedIndex)
     this._addLiveProperty('mode', mode)
 
     if (typeof change === 'function') {
-      this.onInteraction = (details) => {
+      this._addImmutableProperty('onInteraction', (details) => {
         this._selectedIndex = details.selectedIndex
         change(details.selectedIndex, details.isSelected)
-      }
+      })
     }
   }
 }
@@ -286,7 +304,7 @@ TouchBar.TouchBarScrubber = class TouchBarScrubber extends TouchBarItem {
     if (config == null) config = {}
     const {items, selectedStyle, overlayStyle, showArrowButtons, continuous, mode} = config
     let {select, highlight} = config
-    this.type = 'scrubber'
+    this._addImmutableProperty('type', 'scrubber')
     this._addLiveProperty('items', items)
     this._addLiveProperty('selectedStyle', selectedStyle || null)
     this._addLiveProperty('overlayStyle', overlayStyle || null)
@@ -297,13 +315,13 @@ TouchBar.TouchBarScrubber = class TouchBarScrubber extends TouchBarItem {
     if (typeof select === 'function' || typeof highlight === 'function') {
       if (select == null) select = () => {}
       if (highlight == null) highlight = () => {}
-      this.onInteraction = (details) => {
+      this._addImmutableProperty('onInteraction', (details) => {
-        if (details.type === 'select') {
+        if (details.type === 'select' && typeof select === 'function') {
           select(details.selectedIndex)
-        } else if (details.type === 'highlight') {
+        } else if (details.type === 'highlight' && typeof highlight === 'function') {
           highlight(details.highlightedIndex)
         }
-      }
+      })
     }
   }
 }