fix: ensure created comments create URLs with inspect and comment params #5012
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
robinpyon
commented
Oct 18, 2023
Comment on lines
+57
to
+67
| const activeToolName = useRouterState( | ||
| useCallback( | ||
| (routerState) => (typeof routerState.tool === 'string' ? routerState.tool : undefined), | ||
| [], | ||
| ), | ||
| ) | ||
| const {tools} = useWorkspace() | ||
| const activeTool = useMemo( | ||
| () => tools.find((tool) => tool.name === activeToolName), | ||
| [activeToolName, tools], | ||
| ) |
There was a problem hiding this comment.
This was moved out of useNotificationTarget since the tool name isn't part of the notification object
|
No changes to documentation |
robinpyon
commented
Oct 18, 2023
| * @beta | ||
| * @hidden | ||
| */ | ||
| export interface CommentContext { |
There was a problem hiding this comment.
Exporting this since I wanted to avoid something like NonNullable<CommentDocument['context']>['notification'] as a return type for getNotificationValue – but maybe that's OK
|
Component Testing Report Updated Oct 18, 2023 11:42 PM (UTC)
|
| // We force a value for `inspect` to ensure that this is included in URLs when comments | ||
| // are created outside of the inspector context (i.e. directly on the field) | ||
| // @todo: consider filtering pane router params and culling all non-active RHS panes prior to generating this link | ||
| const path = createPathWithParams({ |
There was a problem hiding this comment.
Nice that this function was useful here as well! 👍
We don't do the "force thing" when copying the comment URL, we should maybe add it there as well.
There was a problem hiding this comment.
Probably unnecessary since you can only (currently) copy from the document inspector, but also probably wouldn't hurt to include!
hermanwikner
approved these changes
Oct 19, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 20, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 23, 2023
skogsmaskin
pushed a commit
that referenced
this pull request
Oct 23, 2023
skogsmaskin
pushed a commit
that referenced
this pull request
Oct 24, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 24, 2023
skogsmaskin
pushed a commit
that referenced
this pull request
Oct 24, 2023
skogsmaskin
pushed a commit
that referenced
this pull request
Oct 24, 2023
skogsmaskin
pushed a commit
that referenced
this pull request
Oct 25, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 26, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 27, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 30, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 31, 2023
hermanwikner
pushed a commit
that referenced
this pull request
Oct 31, 2023
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]>
stipsan
pushed a commit
that referenced
this pull request
Nov 6, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR ensures created comments store URLs based on current
window.location.originplus the current path based off current pane params.This is similar to how the current 'copy link to comment' works - the only difference is that we force
inspectandcommentparams when comments are created directly on fields.There is a minor point to note which is when links are created on active panes that are not the most-RHS desk pane. It's outlined in comments but worth mentioning. E.g. if you have 5 panes open and the second one is active and you create a comment on it, it'll create a link containing the full pane layout.
We can probably improve on this in future and look to filter out non-active panes (beyond the current active one). The same logic could potentially be applied to 'copy link to comment' too – but the current implementation may be good enough for now.
What to review
Create a comment (mention / reply) and wait for the notification email. Clicking the "View in Studio" link should take you directly to the comment with the inspector open.