-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
slate-hyperscript tests and decorations #1777
slate-hyperscript tests and decorations #1777
Conversation
<value> | ||
<document> | ||
<paragraph> | ||
This is one <highlightAnchor key="a" />block. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the decoration key
used for?
Cannot you use the decoration key
as a way to pair the start/end of the decoration?
If not, I would suggest the following syntax:
<highlight anchor key="a"/>
<highlight focus key="a" />
<highlight anchor key="a"/>
<highlight focus key="a"/> Thanks, that's a much better syntax suggestion, and I've updated to match it. These two updated tests help illustrate why the
|
Huh, the CI build failed because of this error in upath (on the latest version of node) which suddenly hit us, unrelated to the PR: |
Well it looks like we can still tell which decoration edges pair together,
just using the key. Following the document order, the first with key a is
the anchor, the second with key a is the focus
… |
Misunderstood your original question; I see, you're not asking if we can do away with the keys, but if we can do away with the anchor / focus props. That is true, I believe, assuming any self-closed decoration tag with a key would then just be treated as an endpoint to pair up with its next matching key. I can refactor that way I believe. |
Updated to drop anchor / focus, although the Travis build is still broken due to #1795 |
@jasonphillips do you know what should be done about |
I suppose since this is really a browser-focused library (except perhaps for the de/serialization packages that I do use on the server a bit), "officially" testing it on cutting-edge node versions in CI isn't necessarily something that matters? In which case, just setting Travis to use the prior stable Node certainly seems reasonable to me for now. |
@jasonphillips this is absolutely amazing. These tests look great, and the ability to model decorators with hyperscript is super smart! I didn't realize that the two PRs would conflict, and I've already merged the other. Would you be able to resolve the conflicts and then I can merge this? Thanks so much for your help. |
No problem -- latest master merged & conflicts resolved. I moved the 3 inline tests from the other PR into the test directories of this one. |
@jasonphillips thank you! Moving those is above and beyond, thank you very much. |
* initial simple decorations (mark-like), many tests added * allow decorators to be set by focus, anchor tags - add tests * handle one more edge case with decorations in hyperscript * apply prettier cleanup * apply linting rules * update changelog * ensure always normalize decoration ranges * reapply prettier after latest adjustments * all operations apply now update decorations with selection * ranges can now be 'atomic', will invalidate if contents change * lint, prettier cleanups * add atomic invalidation tests, update hyperscript usage * fix linter errors * minor cleanup * slight refactor for simplicity * remove a couple superfluous lines * update in response to review * drop unnecessarily committed add'l file * remove the need for explicit anchor, focus prop on decoration tags * update hyperscript use to match latest syntax in #1777 * atomic -> isAtomic
This PR first adds tests to
slate-hyperscript
, covering all its basic functionality as well as its handling of selection (by cursor, and by anchor / focus).It then adds the ability to similarly notate decorations in hyperscript, using either a syntax similar to marks (if it's a simple range within a continuous section of text), or using a
[decoration]Anchor
/[decoration]Focus
syntax.For the simple syntax, see this test
For the Anchor / Focus syntax (must supply matching key since can be multiple), see this test
I dove into these improvements mainly as a preparation for being able to more rigorously test decorations (I'm working on solving #1291 ), which is greatly helped when
slate-hyperscript
can easily include decorations in its readable notation.