Move attribute element highlight code from link to typing #7546
Conversation
rename findLinkRange to findAttributeRange, so it could be shared between attributes.
copy them from linkediting, and adapt to no-plugin case.
There was a problem hiding this comment.
The docs should have better wording - just details but might better understand the feature and should not mix view/model terminology.
Other than that I'm thinking about changing the API of findAttributeRange() as it's usage isn't that universal and always requires the same steps and code to be used which might be handled inside that function.
Correct me if I'm wrong, but I think this feature in general mixes the view and the model. That said, I totally support making the docs clearer, but just probably lack the ideas how :) BTW, I was trying to make simply move the code, and make minimal number of changes to its behavior (=> docs) to support multi-attribute 2SCM. |
as suggested at #7546 (comment).
|
@jodator I think it's ready for another look. |
|
@tomalec Sorry, I've forgot to mention that some other PR was using |
|
@jodator Fixed, I hope it's fine now. |
|
I've found one issue with the docs only. The rest comments were addressed 👍 |
# Conflicts: # packages/ckeditor5-link/src/linkediting.js
Make the code less link-specific, so it could be used for other attributes.
Suggested merge commit message (convention)
Other: Link's attribute element highlight is now a public utility.
Additional information
This is to address #7444 and #6722.
I do not like the redundancy in tests for highlighting. I copied the set of tests from
linkeditingtoinlineHighlight. I wanted to remove part of them fromlinkEditingand just check some high-level behavior and thatlinkusesinlineHighlight. Unfortunately, it's also not a perfect approach, see https://cksource.slack.com/archives/C03URJ97S/p1593637492006600I'd leave it as it is, just to keep code coverage happy, and move on with the functional change.