fix: make autoclose toggleable for flyouts #7634
Conversation
| @@ -426,7 +429,7 @@ export abstract class Flyout extends DeleteArea implements IFlyout { | |||
| */ | |||
| dispose() { | |||
| this.hide(); | |||
| this.workspace_.getComponentManager().removeComponent(this.id); | |||
| this.targetWorkspace.getComponentManager().removeComponent(this.id); | |||
There was a problem hiding this comment.
Was this just a bug before? It does seem odd to remove it from the component list of its own workspace which we'll dispose of anyway in just a few lines.
There was a problem hiding this comment.
Yesh this was indeed a bug before.
| @@ -382,22 +382,10 @@ export class HorizontalFlyout extends Flyout { | |||
| } | |||
| } | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
It makes way more sense for this logic to be in the metrics manager, so I like that change. But I'm wondering if it has the possibility to break people who depend on the current values the metrics manager returns / will they have to update their math to account or not account for the flyout now? or was the translate added by the flyout already accounted for in the metrics?
There was a problem hiding this comment.
The translation gets overwritten the next time the metrics are applied to the workspace. So we were usually overwriting this update anyway.
We definitely /could/ break people that are depending on the current metrics values. But it's unlikely to do that because we're essentially adding new functionality. We have two new possible states which are categories + always open flyout and no categories + hiding flyout, and we're properly reporting those metrics.
There was a problem hiding this comment.
We should definitely manually test whether this breaks the continuous flyout though. I created a testing task in the GH project and assigned it to Rachel since she's doing release stuffs.
The basics
The details
Resolves
Fixes #7506
Also google/blockly-samples#1922 ? Need to check.Proposed Changes
Makes it so that autoclose on flyouts is toggleable.
Reason for Changes
People want to make the continuous toolbox autoclose. Also app inventor wants to control the flyout from an external component, so they use an autoclosing simple flyout.
Test Coverage
Manually tested steps here.
Fixed up some change detector unit tests.
Documentation
N/A
Additional Information
N/A
Makes google/blockly-samples#1922 possible to do further work on. Still needs fixes to
refreshSelection