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

fix(comments): only calculate current caret element on demand #4935

Merged
merged 1 commit into from
Sep 15, 2023

Conversation

robinpyon
Copy link
Contributor

Description

A temporary fix which only requests the element at the current caret on demand rather than listening to all selectionchange events.

This exposes activeCaretElement on CommentInputProvider, and will be replaced with an alternate implementation in the near future (which selectively listens only when the mentions menu is open).

@vercel
Copy link

vercel bot commented Sep 15, 2023

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

Name Status Preview Updated (UTC)
performance-studio ✅ Ready (Inspect) Visit Preview Sep 15, 2023 9:17am
test-studio ✅ Ready (Inspect) Visit Preview Sep 15, 2023 9:17am
1 Ignored Deployment
Name Status Preview Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Sep 15, 2023 9:17am

@github-actions
Copy link
Contributor

No changes to documentation

@github-actions
Copy link
Contributor

Component Testing Report Updated Sep 15, 2023 9:23 AM (UTC)

File Status Duration Passed Skipped Failed
formBuilder/ArrayInput.spec.tsx ✅ Passed (Inspect) 10s 3 0 0
formBuilder/inputs/PortableText/Annotations.spec.tsx ✅ Passed (Inspect) 10s 3 0 0
formBuilder/inputs/PortableText/Decorators.spec.tsx ✅ Passed (Inspect) 11s 6 0 0
formBuilder/inputs/PortableText/Input.spec.tsx ✅ Passed (Inspect) 15s 9 0 0
formBuilder/inputs/PortableText/ObjectBlock.spec.tsx ✅ Passed (Inspect) 51s 18 0 0
formBuilder/inputs/PortableText/Styles.spec.tsx ✅ Passed (Inspect) 12s 6 0 0
formBuilder/inputs/PortableText/Toolbar.spec.tsx ✅ Passed (Inspect) 6s 3 0 0

Copy link
Member

@hermanwikner hermanwikner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! 🙏

@robinpyon robinpyon merged commit bf14798 into feat/comments-mvi-1 Sep 15, 2023
13 of 14 checks passed
@robinpyon robinpyon deleted the feat/comments-mvi-1-selection-fix branch September 15, 2023 09:43
github-merge-queue bot pushed a commit that referenced this pull request Oct 31, 2023
* feat: comments wip

* add handle to commentslist, scroll down to comment fix

* fix(comments): only calculate current caret element on demand (#4935)

* feat(comments): minor UI updates (#4929)

- Tooltip delays (500 in, 0 out), <TooltipDelayGroupProvider> wraps comment actions
- Optically aligned icons (still temporary, changes forthcoming in @sanity/icons)
- Display total comment (not thread) count per field
- Minor changes to open/resolved selects, comment context menu alignment + field headers
- Primary tone on new comment container, also broken out into a separate component
- Other super minor tweaks, slightly more condensed comments, PTE container
- Bump @sanity/ui to 1.8.2
- Uses current dataset name when querying for metacontent comments

* feat: improve comment breadcrumbs

* feat: disable replying/creating new comments when resolved

* feat: scroll to comment when opening inspector

* feat: use correct feature key in `useCommentsEnabled`

* refactor: scroll to thread, feature enable check, build breadcrumbs

* feat: ui updates

* dev(test-studio): add comments debug schema

* feat: ui updates, refactor, add array item schema title in breadcrumbs

* refactor(comments): inline text filtering for mentions menu

* refactor(comments): remove unused 'expanded' context

This is no longer in use it seems like. Removing for simplicity.

* refactor(comments): introduce focus to end of content as explicit fn

Make a explicit function to focus the editor at the end of the
edited comment content as focusEditor fn is now used internally
in the context provider.

Also simplify an effect that focuses the component this way.

* fix(comments): fix issue with focusing editor through effect

Make sure the editor is mounted before trying to call focus on it.

* feat: validate comments against schema and document value

* dev(test-studio): update comments debug schema

* feat: improve comment breadcrumbs

* refactor(comments): remove unused 'expanded' context

This is no longer in use it seems like. Removing for simplicity.

* fix: resolve conflicts

* fix: scroll to comment

* refactor: types, add comments, clean up

* fix: delete comment dialog message

* feat: update pte list config

* fix(comments): create better placement for mentionmenu popover

* fix: move static props outside the render scope, a11y improvements

* feat: add comment creation error handling

* dev(test-studio): update comments debug schema

* fix: sort order when creating a new thread + scroll to comment issues

* refactor: minor improvements

* feat: improve perf by memoizing the comments list component

* fix: add proper tags to comments list

* feat: cache system groups response to prevent unnecessary requests

* fix: breadcrumbs build

* test: build comment breadcrumbs

* fix(core/comments): ensure proper authentication config on comment client

Re-use the authentication configured for the default client.
Authentication method vary if the browser supports cookies or not.

* dev(test-studio): temporarily disable `assist` and `tsdoc` plugins

* refactor: move comments from `core` to `desk`

* feat: add `createPathWithParams` to pane router

* feat: implement comments setup

* feat: add `useCommentsSetup` hook

* feat: add `client` option to `useCommentsStore` and `useCommentOperations`

* refactor: remove unused context

* test: update `buildCommentBreadcrumbs` test

* feat: add copy link to comment feature, UI tweaks and clean up

* refactor: export `useMentionOptions` options interface

* refactor: update comment workshop stories

* fix: attach workspace title to comments notification context (#4992)

* feat: add highlight/scroll logic + general improvements

* fix(comments): handle text overflow in editable surfaces, optically align field titles (#5004)

* fix(comments): add viewport specific max-height to editable comment areas

* fix(comments): optically align thread field titles

* fixup! fix(comments): add viewport specific max-height to editable comment areas

* fix: field highlight effect array issue by using `AnimatePresence`

* feat: implement comment discard logic

* feat: disable comments by default

* dev(test-studio): enable comments

* fix: ensure created comments create URLs with inspect and comment params (#5012)

* chore: add code owner for comments

* fix: re-focus input when cancelling comment discard when creating new thread

* refactor: simplify discard controller

* fix: reset comment id ref from params when scroll completed

* fix: move group to top when new thread is created + select the path

* fix: tweak field highlight overlay

* test(comments): add test-ids

Add a couple of test-ids we will need when writing tests.

* refactor(comments): refactor focus handling in the CommentInput

This will fix some bugs we have been seeing related to setting focus in the comment input.
Use the PTE Editable selection prop to control selecting end of the content.
Call focus on the editor without side-effects.

* test(playwright-ct): add tests for CommentInput

Add some basic tests for the comment input. More to come.

* fix(comments): use current editor value over snapshot to update comment

* test(playwright-ct): update props after refactor

* fix: store document references as plain objects (#5021)

* feat: implement breadcrumbs button

* feat: reset select path when changing status view

* fix: reset select path when closing inspector

* feat: improve selected path logic, update ui when creating new thread

* fix: revert the storing of cross dataset references in comment documents

* feat: add  to notification context (#5041)

* dev(test-studio): remove temporary workspace

* dev(test-studio): re-enable `tsdoc` and `assist` plugins

* chore: update `yarn.lock`

* chore: bump sanity ui

* refactor: remove `useFieldCommentsCount` hook

* feat: handle network connection reconnect in `useCommentsStore`

* feat: disable comments (input and actions) while running setup

* fix: retry comment create issue

* fix: `focusOnMount` issue

* chore: update yarn.lock

* feat: add comments onboarding

* refactor: setup when creating the first comment

* fix: update comments onboarding local storage key

* fix: scroll to comment

* refactor(comments): refactor how input and focus is handled

* Enter will submit the comment
* Input field is reset after submission or discard
* Setting focus is handled through the same function everywhere.

* test(playwrigh-ct): add test for comment submission

* refactor(comments): Performance opt: Don't re-render replies every time.

* refactor(comments): refactor key event handling in the comment input and consumers

This will let comment input key events propagate to the parent components for
handling spesific functionality for that parent.

This also reduces the need for additional key handlers in the parent components,
and makes it easier to compose functionality as there is only one originating source
of those events.

* fix: comments onboarding copy

* refactor: make code more readable in `FormFieldBaseHeader`

* fix: incorrect interface name in `CommentsSetupProvider`

* fix: remove unused CSS in `CommentFieldButton`

* fix: wrap local storage functions in try/catch in `CommentsOnboardingProvider`

* refactor: get rid of `satisfies` where possible

* fix(core): invalid tsdoc in `useDidUpdate`

* fix: add request tag in `CommentsSetupProvider`

* fix(studio-e2e-testing): remove `commentAction` field action

* Update packages/sanity/src/desk/comments/src/context/setup/CommentsSetupProvider.tsx

Co-authored-by: Robin Pyon <[email protected]>

---------

Co-authored-by: Robin Pyon <[email protected]>
Co-authored-by: Per-Kristian Nordnes <[email protected]>
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