Move append to create-append-anything module
#1815
Conversation
smbea
added
backlog
smbea
changed the title
create-append-anything module
smbea
force-pushed
the
move-append-to-module
branch
from
d5ad82b
to
964a7d7
Compare
| AutoPlace | ||
| AutoPlaceModule, | ||
| ContextPadModule, | ||
| EditorActionsModule, |
There was a problem hiding this comment.
We should not be dependent on this, but make it an optional dependency.
| AutoPlaceModule, | ||
| ContextPadModule, | ||
| EditorActionsModule, | ||
| KeyboardModule |
There was a problem hiding this comment.
We should not be dependent on this, but make it an optional dependency.
| * | ||
| * @param {Injector} injector | ||
| */ | ||
| export default function CreateAppendEditorActions(injector, editorActions) { |
There was a problem hiding this comment.
Consider to make this optionally depend on editorActions.
You'll otherwise not be able to properly compose it.
Alternative (but maybe overkill):
- add
create-append-anything/editor-actionsandkeyboardmodules
There was a problem hiding this comment.
One small topic you may want to follow up on to improve composability:
- Do not hard wire keyboard and editor actions as dependencies, as the modeler works without those usually.
Other than that, great! A really good showcase how this improves maintainability to have such feature contained in a modular fashion.
smbea
force-pushed
the
move-append-to-module
branch
from
964a7d7
to
c086014
Compare
|
Thanks for the feedback. It makes a lot of sense. I decided to make them optional dependencies by using |
This is a follow up from #1811 (comment)
Moving this causes what seems like a big PR so let me know if I can help make the review easier in any way.
What I suggest is to check out commits 9f302000 and d5ad82ba which add something new (encapsulating editor actions/keyboard shortcuts). The rest is pretty much the same but on a different location (but of course feel free to check it out)