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

Change the way that headings are applied to elements so that they no longer destroy the parent element by replacing the original tag. #5

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

earlcochran
Copy link

Before if someone attempted to add h1 or h2 a <blockquote> or <li> element, it would wrap the parent element in the selected heading tag. This works fine, until the user attempts to remove the heading, upon which the wrapped element would be replaced with a <p> tag, and the <h1> tag was left in place.

To replicate the bug:

  • Select a <blockquote> or <li> element.
  • Click on h1 to add h1 to the selected element.
  • Attempt to remove h1 from the selected element.

I've fixed it by adding a toggleFormatHeading(tag) function. This function attempts to match the chosen heading tag in the selected elements class names. It removes it if it is found, and adds it if it isn't.

I've also added the proper functionality to reloadMenuState() to properly check for class names so that the right menu action can be highlighted.

You can see the updated functionality by viewing the gh-pages branch from my fork here

Earl Cochran added 2 commits September 1, 2013 11:14
… classes without destroying the parent element
… classes without destroying the parent element
@mduvall
Copy link
Owner

mduvall commented Sep 1, 2013

Thanks for this! I've seen this bug come up and it's awesome that you fixed it. I'll look at the code this afternoon and we'll get this in. 👍

@earlcochran
Copy link
Author

Awesome.

It's a silly bug that would be easily fixed by the heading command in document.execCommand(), but the only browser currently supporting heading is Firefox.

@earlcochran
Copy link
Author

I agree, but without proper support for heading in execCommand(), I'm not sure how else to make it work without destroying other elements. I've been messing around with other solutions, just haven't constructed one better than this yet.

@teemualap
Copy link

I agree with nordbjerg. Css classes should not be the way to go.

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

Successfully merging this pull request may close these issues.

3 participants