panel: trackedPanels caching issue

[fsmeier]

Actual Behavior:

• What is the issue? *

this._trackedPanels[config.id] = panelRef;

config.id is 'undefined' for the first run where no config with id is stored yet.
https://github.com/angular/material/blob/v1.1.5/src/components/panel/panel.js#L1160

• What is the expected behavior?
  The code should use the id in this._config -> this._config.id

AngularJS Versions: *

• AngularJS Version: 1.5.x
• AngularJS Material Version: from the beginning till 1.1.5

Actually this is a bug nobody noticed before and i think it makes no real damage, but it is still a bug.
Before I would make a simple PR I wanted to ask if you see also the problem and if I should make this simple fix the way i described it.

I would like to do my part in stabilizing angularJS material!

Best,
Florian

linking @bradrich as creator of this feature
and @ThomasBurleson as active project-lead of this repo :)

[fsmeier]

Introduced in #9379 @bradrich

[fsmeier]

@Splaktar can you label this pls? - I'll try to make a PR the next two weeks.

[Splaktar]

Please provide a CodePen demo where this exception can be observed.

[Splaktar]

@IMM0rtalis is this addressed in #11133? or is this still an issue?

[fsmeier]

Still an issue, but I think I will not fix this in the next two months. But it is one of my all-time-open-tabs in the browser :D

[Splaktar]

I tried making this change but it breaks 68 tests with Expected 50 to be within 1 of 8 at UserContext or Expected 250 to be within 1 of 8 at UserContext.

For common tests the values of this._config and config are quite different:
For "should retrieve and apply a preset when the preset name is provided during the create or open method",
this._config:

{id: 'panel_0', scope: Scope{$id: 2, $$childTail: null, $$childHead: null, $$prevSibling: null, $$nextSibling: null, $$watchers: null, $parent: Scope{$id: ..., $$childTail: ..., $$childHead: ..., $$prevSibling: ..., $$nextSibling: ..., $$watchers: ..., $parent: ..., $$phase: ..., $root: ..., $$destroyed: ..., $$suspended: ..., $$listeners: ..., $$listenerCount: ..., $$watchersCount: ..., $$isolateBindings: ..., $$asyncQueue: ..., $$postDigestQueue: ..., $$applyAsyncQueue: ...}, $$phase: null, $root: Scope{$id: ..., $$childTail: ..., $$childHead: ..., $$prevSibling: ..., $$nextSibling: ..., $$watchers: ..., $parent: ..., $$phase: ..., $root: ..., $$destroyed: ..., $$suspended: ..., $$listeners: ..., $$listenerCount: ..., $$watchersCount: ..., $$isolateBindings: ..., $$asyncQueue: ..., $$postDigestQueue: ..., $$applyAsyncQueue: ...}, $$destroyed: false, $$suspended: false, $$listeners: Object{$destroy: ...}, $$listenerCount: Object{$destroy: ...}, $$watchersCount: 0, $$isolateBindings: null}, attachTo: Object{0: <div ng-app=""></div>, length: 1}, bindToController: true, clickOutsideToClose: false, disableParentScroll: false, escapeToClose: false, focusOnOpen: true, fullscreen: false, hasBackdrop: false, propagateContainerEvents: false, transformTemplate: function() { ... }, trapFocus: false, zIndex: 80, panelClass: 'preset-container', template: '<div>Hello World!</div>'}

config: {}

For "should create and open a basic panel"
this._config:

{id: 'panel_1', scope: Scope{$id: 4, $$childTail: null, $$childHead: null, $$prevSibling: null, $$nextSibling: null, $$watchers: null, $parent: Scope{$id: ..., $$childTail: ..., $$childHead: ..., $$prevSibling: ..., $$nextSibling: ..., $$watchers: ..., $parent: ..., $$phase: ..., $root: ..., $$destroyed: ..., $$suspended: ..., $$listeners: ..., $$listenerCount: ..., $$watchersCount: ..., $$isolateBindings: ..., $$asyncQueue: ..., $$postDigestQueue: ..., $$applyAsyncQueue: ...}, $$phase: null, $root: Scope{$id: ..., $$childTail: ..., $$childHead: ..., $$prevSibling: ..., $$nextSibling: ..., $$watchers: ..., $parent: ..., $$phase: ..., $root: ..., $$destroyed: ..., $$suspended: ..., $$listeners: ..., $$listenerCount: ..., $$watchersCount: ..., $$isolateBindings: ..., $$asyncQueue: ..., $$postDigestQueue: ..., $$applyAsyncQueue: ...}, $$destroyed: false, $$suspended: false, $$listeners: Object{$destroy: ...}, $$listenerCount: Object{$destroy: ...}, $$watchersCount: 0, $$isolateBindings: null}, attachTo: Object{0: <div ng-app=""></div>, length: 1}, bindToController: true, clickOutsideToClose: false, disableParentScroll: false, escapeToClose: false, focusOnOpen: true, fullscreen: false, hasBackdrop: false, propagateContainerEvents: false, transformTemplate: function() { ... }, trapFocus: false, zIndex: 80, template: '<div>Hello World!</div>'}

config: {template: '<div>Hello World!</div>'}

[Splaktar]

There is certainly something funky here.

'this._config.id: panel_2'
'config.id: undefined'

'this._config.id: panel_0'
'config.id: undefined'

'this._config.id: panel_26'
'config.id: undefined'

this._trackedPanels[undefined] = panelRef; seems to be passing the tests but something like this._trackedPanels['panel_0'] = panelRef; causes failures.

[Splaktar]

Ugh, it looks like my build or browser cache got broken somehow. I cleared and restarted things and now this change isn't causing failures. 😕

[Splaktar]

It looks like this caching was working for tooltips (config.id === md-tooltip-1746), but not panels (config.id === undefinfed).

'this._config.id: panel_978'
'config.id: undefined'
'this._config.id: panel_979'
'config.id: undefined'
'this._config.id: md-tooltip-1720'
'config.id: md-tooltip-1720'
'this._config.id: md-tooltip-1721'
'config.id: md-tooltip-1721'