adobe/brackets — pull request #4421 by njx (2013-07-11 01:15:53+00:00)
Title: INITIAL REVIEW: Animate inline editors open.

Makes it so inline editors slide open and closed smoothly. Avoids various race conditions that can occur while inline editors are opening or closing.

While this seems to be working well, there are a few things I'm not sure about, mostly related to cleanups I think we should eventually do to the inline editor stuff.

* In order for the animation to look smooth on retina, I force it to use the GPU by setting `transform: translateZ(0)` on the inline editor during the animation. However, that causes the entire window to flicker sometimes when an animation completes. The hack of setting `backface-visibility: hidden` on `body` appears to fix this. I don't *think* this causes the entire window to use the GPU (I don't see any performance issues with it set vs. unset), but we should verify that.
* Because opening an inline editor now completes asynchronously, we return a promise that's resolved when the animation is done. The open animation is triggered by the inline editor calling `Editor.setInlineWidgetHeight()` once on its host editor. That's sort of weird from an API point of view--if the implementor of the inline editor doesn't know s/he needs to call this, the editor will never open and the promise will never resolve. I think it might already have been true that all inline editors needed to call this function even before adding the animation, so this isn't necessarily a new problem, but it does make me think we should clean up the API here--maybe requiring `onAdded()` to return a desired initial height?
* The initialization lifecycle of inline editors isn't clear. There's the constructor, `load()`, and `onAdded()`. Both `load()` and `onAdded()` are theoretically part of the external API (they're stubbed in InlineWidget), but `load()` is only ever used by inline editor providers themselves immediately after calling the constructor; it's never called by Editor (only `onAdded()` is). On top of that, it seems like there was at least one bug where something that happens in `MultiRangeInlineEditor.load()` that used to work when everything was synchronous now needs to happen in `onAdded()` instead (though it's not clear why). I added a workaround for that specific case, but it seems like we need to be clear about what should happen at construction time (before being added to the DOM) vs. after being added.

The question is whether we should do those cleanups now, or have a larger inline editor architecture cleanup that would deal with some of the other architectural messiness we've had for awhile as well (like InlineTextEditor, which is a sort of vestigial subclass that no one uses directly).