Clean up addWidget flow#535
Conversation
…nlineWidget just takes an actual inlineWidget and stores the widget itself in the _inlineWidgets list.
|
This actually fixes #531 which I had filed earlier. |
|
Reviewing |
There was a problem hiding this comment.
Return value no longer used.
|
Initial review complete |
|
Responded to code review. I considered changing the names of onAdded() and onParentShown() as well, but those really do feel like they're reacting to events, as opposed to close(), which feels like it's really disposing the content. Kind of a fine distinction, though. |
|
Looks good except for InlineTextEditor changes in the most recent commit. |
|
Ugh, my bad. I think we should revert the original close handler's name back to onClosed()--there might be some cleaner way to rename that vs. this close method, but I think we could tackle that later. |
|
OK, I reverted back to onClosed(). Please take a look. |
|
Looks good. But need to merge with master. |
|
Merged with master. Ran the unit tests and looks like everything's still working. |
Fix adobe#530 - Figure out why DnD using the Upload Files dialog is broken
2 participants
@jason-sanjose @tvoliter @peterflynn -- one of you guys should take a look at this one :)
Currently, we have EditorManager._addInlineWidget(), which takes an InlineWidget instance, and Editor.addInlineWidget, which takes a bunch of arguments (including an InlineWidget and some methods on that widget) and stores off an anonymous object that basically just points to properties of the InlineWidget. This pull request consolidates everything so Editor just maintains a list of InlineWidget instances, and makes it so both Editor.addInlineWidget() and Editor.removeInlineWidget() take a widget instead of an id. This simplifies the code considerably.