Added read-only mode to the ImageTextAlternative.
#124
Conversation
src/imagetextalternative.js
Outdated
|
|
||
| const panel = new ImageBalloonPanel( editor ); | ||
| const form = new TextAlternativeFormView( editor.locale ); | ||
|
|
||
| form.labeledInput.inputView.bind( 'isReadOnly' ).to( command, 'isEnabled', value => !value ); | ||
| form.saveButtonView.bind( 'isEnabled' ).to( command ); |
There was a problem hiding this comment.
Shouldn't it rather be form.bind( 'isEnabled' ).to( command ); and then, inside TextAlternativeFormView, isEnabled should be propagated among the inputs and buttons?
There was a problem hiding this comment.
This way the form will be re–usable with all its features. In your implementation, if you want to re–use the form, you got to repeat the code from the lines above each time.
There was a problem hiding this comment.
If the form had 15 controls, this would become a DRY madness.
There was a problem hiding this comment.
That was my first thought too. I'll change it.
There was a problem hiding this comment.
I remember why I didn't do it. Because LinkFormView is bind to two commands LinkCommand and UnlinkCommand. But image alt use only one command so can have a common TextAlternativeFormView#isEnabled state.
There was a problem hiding this comment.
I'm not sure about it. TextAlternativeFormView is not a reusable view.
There was a problem hiding this comment.
Maybe we should create some generic FormView? This view might know its children and easily disable and enable it.
|
I agree with https://github.com/ckeditor/ckeditor5-image/issues/123#issuecomment-313639614. Text alternative is not something that needs to be available in read-only mode. I'm for closing this PR. |
Suggested merge commit message (convention)
Feature: Added read-only mode to the
ImageTextAlternative. Closes ckeditor/ckeditor5#5108.Requires ckeditor/ckeditor5-ui#267.