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

No active state for button #17

Open
tmconnect opened this issue Jan 18, 2017 · 9 comments
Open

No active state for button #17

tmconnect opened this issue Jan 18, 2017 · 9 comments

Comments

@tmconnect
Copy link

After clicking a button it's not changed as active. Also it's not possible to delete the formatting of the selected text by clicking the button again.

Please change this behavior so the buttons would work as the standard button.

Thanks!

@boesie
Copy link

boesie commented Feb 3, 2017

Yes, we also have this problem.. How can we fix this?

@arcs-
Copy link
Owner

arcs- commented Feb 5, 2017

Can you please provide the code you used to create this issue?

@boesie
Copy link

boesie commented Feb 10, 2017

If you hit the button for example 10 times. you get:

<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
<span class="class1">
**EXAMPLE WORD**
</span>
</span>
</span>
</span>
</span>
</span>
</span>
</span>
</span>
</span>

So this is not useful, I hope you can help us with this.

@disciple-dev
Copy link

I'm also running into this issue; here's the source I'm using:

    new MediumEditor(document.getElementById('blog_content'),{
      buttonLabels: 'fontawesome',
      toolbar: {
        buttons: ['bold', 'italic', 'underline', 'strike', 'anchor', 'h2', 'h3', 'quote']
      },
      extensions: {
        'strike': new MediumButton({
          label: '<i class="fa fa-strikethrough"></i>',
          start: '<s>',
          end: '</s>'
        })
      }
    })

When I attempt to add and then remove the strikethrough, here's the resulting markup:

<p>
  <s>
    <s>
      <s>4567890</s>
    </s>
  </s>
</p>

If I keep clicking the button, it keeps adding additional tags, as @boesie mentioned.

@arcs-
Copy link
Owner

arcs- commented Mar 2, 2017

I've noticed the problem and am trying to find a solution for it. Unforgenetly I haven't had any success yet.

@disciple-dev
Copy link

I had a look at this a couple of days ago.

TL;DR: A more robust solution for returning selected text is necessary.

The problem seems to be that getCurrentSelection() doesn't actually grab the underlying HTML when you select only the part you want to affect.

If I have added my implementation of the bold button from above, the way to remove the styling as this library sits is to select additional text around the styled text. That is, if I have the text:

<p>
    Progress isn't made by early risers. It's made by <code>lazy men</code> trying to find easier ways to do something.
</p>

Then selecting 'lazy men' and calling getCurrentSelection() won't select <code>lazy men</code>, it only returns the text as rendered.

But if you selected 'by lazy men trying', since the <code> tags are within the selection, it works. I guess this is why timdown has gone on to create rangy.

@arcs-
Copy link
Owner

arcs- commented Mar 10, 2017

That is where I started and actually have almost found a solution for the getCurrentSelection() method. There is only one case left where my concept fails. Unforgenetly further improvements will have to be made as well. I'll update this here in a few days

Concerning Rangy, even though it could work, I'd prefer not to use another library.

@arcs-
Copy link
Owner

arcs- commented Mar 17, 2017

I'm still working on the replacement, this is my current approach, it has one mistake left, that the start/end locations sometimes are wrong.

function getCurrentSelection() {
    var sel = window.getSelection()

    if (!sel.rangeCount) return

    var range = sel.getRangeAt(0)
    var ancestor = range.commonAncestorContainer
    var parentValue = ancestor.nodeValue || ancestor.innerHTML

    var selected = parentValue.substring(range.startOffset, range.endOffset)

    if (parentValue.trim() === selected.trim()) {
        return ancestor.parentNode.outerHTML
    } else {

        if (range.startOffset === 0) {
            var outerHTML = ancestor.parentNode.outerHTML
            var withBegin = outerHTML.substring(0, outerHTML.indexOf(selected) + selected.length)
            return withBegin
        }

        if (range.endOffset === ancestor.length) {
            var outerHTML = ancestor.parentNode.outerHTML
            var withEnd = outerHTML.substring(outerHTML.lastIndexOf(selected))
            return withEnd
        }
		
        var container = document.createElement('div')
        container.appendChild(range.cloneContents())
        var containerHTML = container.innerHTML
		
	//if(containerHTML.charAt('0') === '<') containerHTML.substring(containerHTML.indexOf('/>'))
        //if(containerHTML.charAt(containerHTML.length) === '>') containerHTML.substring(containerHTML.lastIndexOf('<'))
		
		
        return container.innerHTML

    }

}

@ahmadyousefdev
Copy link

Any progress ?

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

No branches or pull requests

5 participants