Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

“Remove highlight” button is not following the highlight command disabled state #9174

Closed
serhiiyanhol opened this issue Mar 5, 2021 · 7 comments · Fixed by #9250
Closed
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:highlight squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.

Comments

@serhiiyanhol
Copy link

serhiiyanhol commented Mar 5, 2021

📝 Provide detailed reproduction steps (if any)

  1. Open highlight feature guide and scroll to "Inline buttons" sample.
  2. Disable the highlight command. You can do this by executing the following code document.querySelectorAll( '.ck-editor__editable' )[ 2 ].ckeditorInstance.commands.get( 'highlight' ).forceDisabled( 'someId' ).

✔️ Expected result

On disable Highlight plugin buttons in toolbar should become inactive.

❌ Actual result

“Remove highlight” button is still active.

image

📃 Other details

  • First affected CKEditor version: 24
  • Installed CKEditor plugins: default highlight

Possible solution:

_addRemoveHighlightButton() {
const t = this.editor.t;
const command = this.editor.commands.get( 'highlight' );
function decorateHighlightButton( button ) {
button.bind( 'isEnabled' ).to( command, 'isEnabled' );
}
this._addButton( 'removeHighlight', t( 'Remove highlight' ), eraserIcon, null, decorateHighlightButton );
}

@serhiiyanhol serhiiyanhol added the type:bug This issue reports a buggy (incorrect) behavior. label Mar 5, 2021
@Mgsy
Copy link
Member

Mgsy commented Mar 8, 2021

Hi, thanks for the report. Could you please provide steps to reproduce the problem, as I'm not sure if I understand your case properly?

@Mgsy Mgsy added the pending:feedback This issue is blocked by necessary feedback. label Mar 8, 2021
@Reinmar
Copy link
Member

Reinmar commented Mar 8, 2021

Isn't it a dup of #9021? Which is a dup of some other ticket :D I suppose everyone's annoyed by the same beavior.

@yanholserhii
Copy link

It isn't a dup. Please see the screenshot. In _addHighlighterButton the possibility to disable is provided. But there isn't such possibility for _addRemoveHighlightButton.

image

@yanholserhii
Copy link

I already create PR. Soon I will provide needed test coverage.

@Reinmar
Copy link
Member

Reinmar commented Mar 9, 2021

Sorry for the previous comment. These are indeed separate issues.

Excuse me the ignorance once again (I'm not checking the code), but just to be sure – the issue you reported is that the button is not disabled when the command is disabled. In other words – disabling the command should also disable the button?

@Reinmar Reinmar added this to the nice-to-have milestone Mar 9, 2021
@yanholserhii
Copy link

Do not worry.
Yes, you are right, disabling the command should also disable the button.
We use highlight and removeHighlight separately. So that's why the problem occurred.

@mlewand mlewand added support:2 An issue reported by a commercially licensed client. domain:ui/ux This issue reports a problem related to UI or UX. squad:core Issue to be handled by the Core team. and removed pending:feedback This issue is blocked by necessary feedback. labels Mar 12, 2021
@mlewand
Copy link
Contributor

mlewand commented Mar 15, 2021

There is a PR for this but it's blocked by CLA.

@mlewand mlewand changed the title make “Remove highlight” button able to disable “Remove highlight” button is not following the highlight command state Mar 16, 2021
@mlewand mlewand changed the title “Remove highlight” button is not following the highlight command state “Remove highlight” button is not following the highlight command disabled state Mar 16, 2021
@mlewand mlewand modified the milestones: nice-to-have, iteration 41 Mar 16, 2021
mlewand added a commit that referenced this issue Mar 16, 2021
Fixed (highlight): The remove highlight button now also gets disabled along with the main highlight command. Closes #9174.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:ui/ux This issue reports a problem related to UI or UX. package:highlight squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. type:bug This issue reports a buggy (incorrect) behavior.
Projects
None yet
5 participants