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

feat(comments): introduce inline commenting #5606

Merged
merged 50 commits into from
Mar 1, 2024
Merged

Conversation

hermanwikner
Copy link
Member

@hermanwikner hermanwikner commented Jan 30, 2024

Description

This pull request adds the capability for inline text commenting within block content. Users can now highlight a specific segment of text and attach a comment to it.

Screenshot 2024-02-22 at 15 43 33

What to Review

  • Ensure you can add an inline comment.
  • Verify that the text associated with a comment is displayed within the comment item.
  • Check that the comment highlight adjusts to the commented text when the text is modified.
  • Confirm that the comment highlight is removed when the comment is deleted or resolved. Additionally, ensure that the comment item indicates when the commented text has been deleted by making the quote border gray and displaying a 'link removed' icon with a tooltip for further information.
  • Ensure scrolling to the comment item in the inspector is possible when clicking on text that has a comment.
  • Ensure scrolling to the commented text in the editor is possible when clicking on a comment item.
  • Verify that adding a comment to text that already has a comment is not possible. In this iteration, overlapping comments should not be allowed.

Gotchas

  • Currently, if text that has a comment is deleted and then restored by undoing, the comment will not reapply to the text.

Testing

  • End-to-end testing for the creation of inline comments
  • Various unit tests for the logic that calculates the ranges that should be highlighted in the text

Notes for Release

Users can now make comments on specific text selections within block content

@hermanwikner hermanwikner requested a review from a team January 30, 2024 14:32
@hermanwikner hermanwikner requested a review from a team as a code owner January 30, 2024 14:32
@hermanwikner hermanwikner requested review from skogsmaskin and rexxars and removed request for a team January 30, 2024 14:32
@hermanwikner hermanwikner marked this pull request as draft January 30, 2024 14:32
Copy link

vercel bot commented Jan 30, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
performance-studio 🛑 Canceled (Inspect) Mar 1, 2024 4:20pm
test-studio 🛑 Canceled (Inspect) Mar 1, 2024 4:20pm
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Mar 1, 2024 4:20pm

Copy link
Contributor

github-actions bot commented Jan 30, 2024

Package Documentation Change
@sanity/portable-text-editor +5%
Full Report
@sanity/portable-text-editor
This branch Next branch
24 documented 23 documented
44 not documented 44 not documented
@sanity/migrate
This branch Next branch
17 documented 17 documented
75 not documented 59 not documented
sanity
This branch Next branch
179 documented 179 documented
851 not documented 860 not documented
@sanity/diff
This branch Next branch
13 documented 13 documented
16 not documented 16 not documented
@sanity/block-tools
This branch Next branch
4 documented 4 documented
9 not documented 9 not documented
@sanity/types
This branch Next branch
55 documented 55 documented
233 not documented 239 not documented
sanity/desk
This branch Next branch
84 documented 84 documented
64 not documented 64 not documented
@sanity/mutator
This branch Next branch
7 documented 7 documented
3 not documented 4 not documented
@sanity/cli
This branch Next branch
1 documented 1 documented
31 not documented 31 not documented
sanity/structure
This branch Next branch
2 documented 2 documented
8 not documented 8 not documented
@sanity/util/concurrency-limiter
This branch Next branch
1 documented 1 documented
0 not documented 0 not documented
@sanity/util/legacyDateFormat
This branch Next branch
0 documented 0 documented
4 not documented 5 not documented
@sanity/schema/_internal
This branch Next branch
0 documented 0 documented
12 not documented 12 not documented
@sanity/util/paths
This branch Next branch
1 documented 1 documented
15 not documented 15 not documented
sanity/router
This branch Next branch
17 documented 17 documented
26 not documented 26 not documented
@sanity/schema
This branch Next branch
0 documented 0 documented
2 not documented 2 not documented
sanity/cli
This branch Next branch
2 documented 2 documented
0 not documented 0 not documented
@sanity/vision
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
@sanity/util/fs
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
sanity/_internal
This branch Next branch
0 documented 0 documented
1 not documented 1 not documented
@sanity/util/client
This branch Next branch
1 documented 1 documented
0 not documented 0 not documented
@sanity/util/createSafeJsonParser
This branch Next branch
1 documented 1 documented
0 not documented 0 not documented
sanity/_internalBrowser
This branch Next branch
0 documented 0 documented
3 not documented 3 not documented
@sanity/util/content
This branch Next branch
1 documented 1 documented
5 not documented 5 not documented

Copy link
Contributor

github-actions bot commented Jan 30, 2024

Component Testing Report Updated Mar 1, 2024 4:04 PM (UTC)

File Status Duration Passed Skipped Failed
comments/CommentInput.spec.tsx ✅ Passed (Inspect) 31s 15 0 0
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 6s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 11s 3 0 0
formBuilder/inputs/PortableText/copyPaste/CopyPaste.spec.tsx ✅ Passed (Inspect) 12s 4 2 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 12s 6 0 0
formBuilder/inputs/PortableText/FocusTracking.spec.tsx ✅ Passed (Inspect) 32s 15 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 1m 0s 15 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 1m 0s 18 0 0
formBuilder/inputs/PortableText/RangeDecoration.spec.tsx ✅ Passed (Inspect) 12s 6 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 13s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 19s 9 0 0

hermanwikner and others added 18 commits March 1, 2024 16:52
* Fixes an issue where adding text inside single word ranges didn't expand correctly
* Fixes an issue where bolding partially inside a range expanded the range incorrectly
* Replaces child and comment range start/end indicators with unused utf-8 chars to avoid conflicts with user content
Have each test in it's own file to make them easier to write and read.
Ditch snapshot testing and test against hard coded values that makes
it easier to undstand what is going on.
Improve invalidation of range decorations when text is edited
Improve how we move and update range decorations when content is edited
@hermanwikner hermanwikner marked this pull request as ready for review March 1, 2024 15:55
@skogsmaskin skogsmaskin self-requested a review March 1, 2024 15:55
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.

2 participants