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

protect headers from newline #1074

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

protect headers from newline #1074

wants to merge 1 commit into from

Conversation

rinfex-holfer
Copy link

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
New tests added? not yet
Fixed tickets #1060
License MIT

Description

This is temporary fix for #1060 issue, I did it for a project in which I participate. It must be imporoved of course, because this bug involves not only <h*> elements, but every block-type elements.
It will be good if someone will give ideas for improvements and tests for this PR, I'm afraid to break something).

@nmielnik
Copy link
Member

@Greeny7 thanks for opening this and apologies for the delay in getting some feedback.

  1. Take a look at this part of content.spec.js. There you can find some example tests that test some of the code around where your change is. I would definitely recommend adding some tests to further verify the functionality you're adding. To run the tests, just run grunt from the command line.
  2. Since you're calling getClosestBlockContainer() to get the first block element, you might be able to rip out the existing code that checks if the node var is a block element or part of an <li>. You may just be able to get the element returned from calling getClosestBlockContainer() and see if you get a result other than the editor element itself. If it's not the editor <div> then you know there was no block container and it's ok to allow the <p> to be inserted. However, there might be some elements that we want to be able to have a nested <p> element inside of it (<blockquote> elements come to mind for some reason) so we might want to think about allowing other block containers to still contain a <p>, but seeing how everything work if you just inspect the block container would definitely be worth it if you're can try it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants