[CLOSED] Clean up addWidget flow #532
Comments
|
Reviewing |
|
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. |
Wednesday Apr 04, 2012 at 01:04 GMT
Originally opened as adobe/brackets#535
@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.
njx included the following code: https://github.com/adobe/brackets/pull/535/commits
The text was updated successfully, but these errors were encountered: