Slate: Maybe toggleMark shouldn't expect perfect equality?

[alexsegura]

Do you want to request a feature or report a bug?

Not sure this is a bug

What's the current behavior?

Currently, when you use toggleMark, you need to pass the _exact_ same object to make it work.

For example:

// Working
transform.toggleMark({ type: 'bold', data: { foo: 'bar' } })
transform.toggleMark({ type: 'bold', data: { foo: 'bar' } })

// Not working
transform.toggleMark({ type: 'bold', data: { foo: 'bar' } })
transform.toggleMark('bold')

What's the expected behavior?

Maybe when a String is passed, we should remove all the marks of that type, no matter the other properties?
Just asking, not sure if this could have side effects.
If you agree, I think I can make a pull request.

[YurkaninRyan]

@ianstormtaylor @alexsegura So I recently encountered this problem when trying to apply a "text-color" mark. I think it makes more sense that remove/setting a mark works purely on for that specific use case, and any use case where you can only have one of a type of mark, but it can have multiple different values in it's data.

If we made this change it would certainly break anyones marks that can be applied more then once, with different data. I personally don't know if that's currently possible, or what use case would require that

[ianstormtaylor]

I think there's an issue in that marks are compared based on the combination of their type and data. So if you apply two different marks to a span of text that have the same type but different data, then you would not be able to specify which to remove.

For example:

{ type: 'comment', { data: { id: 1 }}}
{ type: 'comment', { data: { id: 2 }}}

I'm not sure how to nicely resolve this issue. I agree that depending on the use case a decision in either direction would be annoying. But since the marks have to be part of the operations, at some point it needs to be decided how they are compared. It feels like the current solution allows the most flexibility.

You can always retrieve the marks in the range, and then iterate through them, removing any mark that has the type you want to remove.

[YurkaninRyan]

Would it make sense to remove all of the type if they just supply the type to removeMark, but if they supply data then it must be completely equal?

I'm not sure if that really captures the essence of what removeMark should do because then it's possible to remove more then 1 mark at a time...

[ianstormtaylor]

@YurkaninRyan yup it's definitely possible. We could allow that kind of logic with an extra option potentially, or treat it as the default and make the extra option the strict matching maybe.

[TimYi]

@ianstormtaylor we are also implementing a text-color feature, a remove mark by type method is needed.

[ianstormtaylor]

As of https://github.com/ianstormtaylor/slate/pull/3093 (which was just merged), I believe this issue is no longer applicable, because a lot has changed. I'm going through and closing out any potential issues that are not out of date with the overhaul. Thanks for understanding.