Investigate refactoring PanelMultiView.hidePopup and related methods to animate by default
Categories
(Toolkit :: UI Widgets, task)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox102 | --- | affected |
People
(Reporter: aminomancer, Unassigned)
References
Details
This is a followup to bug 1724622, which refactored the PanelMultiView class methods to support closing animations. This was done by adding an optional parameter animate = false, but it seems reasonable for this parameter to be true by default.
The only uses I'm currently aware of that seem to intentionally avoid animations are command/click events on buttons within panels. Which seems reasonable, it might feel sluggish if the panel didn't close immediately when you click something in it. And it would be another point of disagreement between panels and menupopups, I guess. Since the proton update they look very similar, so if they behaved differently it could feel like jank to some users. But other than that, it seems like most callers want an animation if possible.
I haven't looked that deeply yet, but here's my understanding so far: this._panel.hidePopup(animate) is XULPopupElement::hidePopup(bool aCancel), which lands here and gets us here. So the value of animate is ultimately getting passed as aIsCancel to nsXULPopupManager::FirePopupHidingEvent. Across that whole sequence the parameter isn't inverted or transformed in any way. So, should the default value of this parameter be changed, should it only be changed in the JS methods?
The naming is also a bit confusing. The frequent use of the word "cancel" was kinda misleading, at least in the sense that it's used as the value for the attribute animate. That might lead someone to think the animation is being canceled, but the actual result is just that we animate the panel's opacity without transforming it.
Description
•