Opened 10 years ago
Last modified 9 years ago
#12618 confirmed Bug
Block alignment buttons during upload
Reported by: | Piotr Jasiun | Owned by: | |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | General | Version: | 4.5.0 Beta |
Keywords: | Cc: |
Description
Until upload is done image alignment of the images should not be possible because it ends up with the wrong content if user user image2.
Change History (17)
comment:1 Changed 10 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 10 years ago by
Owner: | set to Piotr Jasiun |
---|---|
Status: | confirmed → assigned |
comment:3 Changed 10 years ago by
Status: | assigned → review |
---|
comment:5 Changed 10 years ago by
Status: | review → review_failed |
---|
The code will bleed when there are no justify commands.
There should be an event witch is called every time there is possibility that state changes and every listener can disable command.
There's a command#state event which you can cancel and change the state back to disabled. I find this much more straightforward solution - you listen on these 4 command's state events and block state changes while the widget is focused.
Finally a manual tc (tags: 4.5.0, tc) is missing.
comment:6 Changed 10 years ago by
comment:7 Changed 10 years ago by
Status: | review_failed → review |
---|
comment:10 Changed 10 years ago by
Status: | review → assigned |
---|
While review we noticed two things:
- This change should touch only uploadimage.
- We need a nicer way to control available alignment options or commands states when element/widget is selected.
comment:11 Changed 10 years ago by
Owner: | Piotr Jasiun deleted |
---|---|
Status: | assigned → confirmed |
comment:12 Changed 10 years ago by
Milestone: | CKEditor 4.5.0 Beta → CKEditor 4.5.0 |
---|
comment:13 Changed 10 years ago by
Version: | → 4.5.0 Beta |
---|
comment:14 Changed 9 years ago by
Milestone: | CKEditor 4.5.0 |
---|
comment:15 Changed 9 years ago by
We decided that solution should be more generic, because this is a problem we have with every widget: some commands does not make sense when widget is focused. Instead of having hardcoded list of disabled commands, this logic should be moved to the widget plugin and list of commands should be in a widget definition property.
I am not sure on what level we should disable commands: this should we a list of commands, toobarGroups or toolbars and if it should be a white- or black-list and I do not remember what we decided. We can not easily decide which command makes sense in each context: about
command should be active always, justify*
should be disabled for upload widget, but *list
(which also works on the paragraph) could be available.
I think we should focus on solving current case which is conflict between upload widget and justify in image2 and do not make too generic solutions. So the widget property should simply have the list of commands which should be disabled when widget is focused:
disabledCommands: [ 'justifyleft', 'justifycenter', 'justifyright', 'justifyblock' ]
Then we could have a more generic solution when we fix #12929 one day. With the single entry point user will be able to easily implement black/whitelisting for commands or toolbarGroups, so whatever he need. One day.
comment:16 follow-up: 17 Changed 9 years ago by
I remember that we were thinking about whitelisting commands that could be enabled. It would be cool, because you more know about stuff that will work with your widget rather than about stuff that won't. At least it seemed so... I realised that you would need to whitelist stuff like "source", "undo", "redo", etc. Otherwise, the editor would disable them all. Sad.
I agree with you @pjasiun on the fact the we should now do the minimum. This won't become a super cool feature anyway, because our commands are not grouped/tagged in any reasonable way. Using toolbar groups is not an option, because commands are not buttons. There's a relation most of the time, but that would be ugly :D.
So, let's do the minimum.
comment:17 Changed 9 years ago by
Replying to Reinmar:
I remember that we were thinking about whitelisting commands that could be enabled. It would be cool, because you more know about stuff that will work with your widget rather than about stuff that won't. At least it seemed so... I realised that you would need to whitelist stuff like "source", "undo", "redo", etc. Otherwise, the editor would disable them all. Sad.
I agree, we were talking about white-listing. But also it is true that all commands like "source", "undo", "redo", "about", "find", all insertion commands, etc. should be enabled. This is why I believe black-listing is the simpler solution, and what we need in this case.
Done. Changes in t/12618.
I had to use
selectionChange
event, because when I tried withwidget.focus
is was changed anyway later byupdateCommandsContext
called fromselectionChanged
.The whole enabling/disabling commands mechanism works pretty bad. They can be, disable because of many reasons (context, mode, readonly) and there is no single mechanism which control the state of the command. There should be an event witch is called every time there is possibility that state changes and every listener can disable command.