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

Not possible to use Block Indent Feature with custom headings #8177

Closed
antoniolucasnobar opened this issue Sep 28, 2020 · 4 comments · Fixed by #8459
Closed

Not possible to use Block Indent Feature with custom headings #8177

antoniolucasnobar opened this issue Sep 28, 2020 · 4 comments · Fixed by #8459
Labels
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

@antoniolucasnobar
Copy link

antoniolucasnobar commented Sep 28, 2020

📝 Provide detailed reproduction steps (if any)

  1. Create custom header as defined in Headings page.
  2. install block indentation plugin, as stated in its page
  3. Select the custom the created custom header
  4. Try to use the indent feature

✔️ Expected result

I exptect to be able to use the block indent feature with Custom Headers

❌ Actual result

Both buttons are never enabled, so I can not use this feature

📃 Other details

Looking into the source, I saw the indent block feature only enable itself to knownElements:

// Enable block indentation by default in paragraph and default headings.
const knownElements = [ 'paragraph', 'heading1', 'heading2', 'heading3', 'heading4', 'heading5', 'heading6' ];

This way, we can not use this feature with any custom Headers.

A simple change can allow that. Changing the above line to:

// Enable block indentation by default in paragraph and default headings.
const options = editor.config.get( 'heading.options' );
const headings = options.map(option => option.model);
const knownElements = headings || [ 'paragraph', 'heading1', 'heading2', 'heading3', 'heading4', 'heading5', 'heading6' ];

and voilá, block indent works for all defined headers. Another option would be to have a config option, but I think it would only to double the config between headers and blockindent.

Maybe we do not need the last line, once the headings plugin always set the heading options. But if the user remove the heading plugin, I do not know the results of that.

Please, let me know if this is the way to fix this. I can make a PR, if you folks approve it. If not, please tell me what should be the solution

  • Browser: Firefox 80.0
  • OS: Linux mint 19.3
  • CKEditor version: 22.0
  • Installed CKEditor plugins: Essentials, Alignment, Paragraph, Heading, List, Bold, Italic, Indent, Outdent

As a final note, the link to see the code at the end of the contribute section of block indent feature is pointing to the font plugin, not the indent plugin.


If you'd like to see this fixed sooner, add a 👍 reaction to this post.

@antoniolucasnobar antoniolucasnobar added the type:bug This issue reports a buggy (incorrect) behavior. label Sep 28, 2020
@Mgsy Mgsy added squad:core Issue to be handled by the Core team. support:2 An issue reported by a commercially licensed client. labels Nov 6, 2020
@Reinmar Reinmar added this to the nice-to-have milestone Nov 9, 2020
@Mgsy
Copy link
Member

Mgsy commented Nov 9, 2020

Hi, indeed, custom headings are not supported by the indent block feature, as heading elements are hardcoded. Your solution seems to be a good direction, so it would be great if you could prepare a PR with fix and tests, our team will be happy to review it. Thank you for your contribution!

@antoniolucasnobar
Copy link
Author

Hello @Mgsy
the PR is ready for someone to look at it.

I do not know to whom should I send the signed CLA. Could you help me with this?

And if you need anything to change in the PR, please let me know.

Best Regards.

@antoniolucasnobar
Copy link
Author

CLA e-signed.

@Mgsy
Copy link
Member

Mgsy commented Nov 17, 2020

CLA e-signed.

Great, thanks! We will review your PR soon :)

jodator added a commit that referenced this issue Nov 19, 2020
Fix (indent): The block indent feature will now work with custom headings. Closes #8177.
@Reinmar Reinmar modified the milestones: nice-to-have, iteration 38 Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

Successfully merging a pull request may close this issue.

3 participants