Resolves #57507 - Allows clicking of Modified In labels in setting editor view #60045
Conversation
|
🤦♂️ That makes way more sense than just assuming the first item in the array |
645f626 to
1251cfd
Compare
| DOM.addStandardDisposableListener(view, DOM.EventType.CLICK, (e: IMouseEvent) => { | ||
| this._onDidClickOverrideElement.fire({ | ||
| targetKey: element.setting.key, | ||
| scope: element.overriddenScopeList[i] |
There was a problem hiding this comment.
Won't this always reference the last item in the list, because it will point to i in the closure scope, which is always incremented to the length of the list?
There was a problem hiding this comment.
Huh, that is a fantastic point, let me test it really quick.
There was a problem hiding this comment.
No, testing locally seems to indicate that when it creates the listener and anonymous function is retains the state of the scoped variable. When I manually pushed in a second option it properly used the correct index on click.
There was a problem hiding this comment.
You're right - seems that this works with let but does not work with var. I can't drop the old school habits.
| template.otherOverridesElement.textContent = `(${otherOverridesLabel}: ${element.overriddenScopeList.join(', ')})`; | ||
| } else { | ||
| template.otherOverridesElement.textContent = ''; | ||
| DOM.append(template.otherOverridesElement, $('span', null, `${otherOverridesLabel}: `)); |
There was a problem hiding this comment.
Can we keep the () around the text?
There was a problem hiding this comment.
Definitely! I'll add it now.
1251cfd to
30fbbd7
Compare
Exposed target changing in the action bar, and moved the
Modified Inlabel to a link. This should properly switch between views and automatically filter the list based on category and label name.