Make rebind of CommandPalette optional #65
Conversation
| container.rebind(CommandPalette).to(TheiaCommandPalette); | ||
| if (container.isBound(CommandPalette)) { | ||
| container.rebind(CommandPalette).to(TheiaCommandPalette); | ||
| } |
There was a problem hiding this comment.
Looks good to me but shouldn't we do a "normal" bind if it is not already bound?
There was a problem hiding this comment.
No, I think it doesn't really make sense to bind it if the CommandPalette isn't bound. If it isn't bound, the diagram editor doesn't want to have one, so we shouldn't then bind the TheiaCommandPalette either. Wdyt?
There was a problem hiding this comment.
Ah right, sorry, I somehow was not thinking about that! You are absolutely right! Or is there a way to bind an "empty" command palette? @tortmayr
planger
force-pushed
the
planger/issues/188
branch
from
3683b78 to
28a4778
Compare
There was a problem hiding this comment.
I retested this change with the current master and everything still works very well. It seems that the popup now also vanishes already when opening the context menu but this may also be due to the Theia update. Nevertheless, everything still works fine, please just remove the last commit as the yarn.lock on master is now the correct one.
planger
force-pushed
the
planger/issues/188
branch
from
edde22d to
28a4778
Compare
|
Thanks @martin-fleck-at! I force-pushed the single-commit without the yarn.lock change. |
Fixes eclipse-glsp/glsp#188