fix: copying mentions as text #6583
Conversation
WalkthroughThe pull request updates several components within the editor. The Markdown extension now disables text transformation on copy, while the mention extension is enhanced with new configuration options, helper methods, and type updates to support detailed entity retrieval and markdown serialization. Additionally, the editor components now integrate a member hook to fetch user details for mentions. Changes
Sequence Diagram(s)sequenceDiagram
participant E as EditorBody
participant CME as CustomMentionExtension
participant CMC as CustomMentionConfig
participant S as MarkdownSerializer
participant U as useMember
E->>CME: Initialize mention (with new options)
CME->>CMC: Configure mention (adding getMentionedEntityDetails & renderText)
CMC->>S: Serialize mention using getMentionDisplayText
E->>U: Call getUserDetails to fetch entity details
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/editor/src/core/extensions/mentions/extension-config.ts (1)
67-67: Consider optimizing the fallback chain for mention display text.The current fallback chain (
display_name→id→entity_identifier) might not be optimal. Consider prioritizingentity_identifieroveridas it's more likely to be a meaningful identifier.- return `@${mentionEntityDetails?.display_name ?? attrs[EMentionComponentAttributeNames.ID] ?? mentionEntityId}`; + return `@${mentionEntityDetails?.display_name ?? mentionEntityId ?? attrs[EMentionComponentAttributeNames.ID]}`;web/core/components/pages/editor/editor-body.tsx (1)
193-193: Consider adding error handling for user details retrieval.While the implementation fixes the mention text copying issue, it might benefit from error handling when user details cannot be retrieved.
Consider wrapping the user details retrieval with error handling:
- getMentionedEntityDetails: (id: string) => getUserDetails(id), + getMentionedEntityDetails: (id: string) => { + const userDetails = getUserDetails(id); + if (!userDetails) { + console.warn(`User details not found for ID: ${id}`); + return null; + } + return userDetails; + },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/editor/src/core/extensions/extensions.tsx(1 hunks)packages/editor/src/core/extensions/mentions/extension-config.ts(2 hunks)packages/editor/src/core/extensions/mentions/extension.tsx(1 hunks)packages/editor/src/core/types/mention.ts(2 hunks)web/core/components/pages/editor/editor-body.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Analyze (python)
🔇 Additional comments (6)
packages/editor/src/core/types/mention.ts (1)
2-2: LGTM! Well-structured type enhancement.The addition of
getMentionedEntityDetailsmethod toTReadOnlyMentionHandlertype is well-designed:
- Clear input/output types
- Optional for backward compatibility
- Properly imports and utilizes
IUserLitetypeAlso applies to: 23-23
packages/editor/src/core/extensions/mentions/extension.tsx (1)
12-12: LGTM! Clean integration of the new method.The
getMentionedEntityDetailsmethod is properly integrated into the extension configuration without affecting existing functionality.Also applies to: 18-18
packages/editor/src/core/extensions/mentions/extension-config.ts (1)
46-48: LGTM! Well-implemented text copying for mentions.The implementation properly handles text copying through:
renderTextmethod for direct text copying- Markdown serialization support
- Consistent text format using
getMentionDisplayTexthelperAlso applies to: 63-68
packages/editor/src/core/extensions/extensions.tsx (1)
140-140: LGTM! Correctly disables markdown transformation for copied text.Setting
transformCopiedTexttofalseallows the mention extension'srenderTextmethod to handle the text format, fixing the HTML copying issue.web/core/components/pages/editor/editor-body.tsx (2)
24-24: LGTM!The addition of
useMemberhook follows the existing import pattern and is correctly grouped with other store hooks.
69-70: LGTM!The
getUserDetailshook extraction is well-placed with other store hooks and maintains consistent code style.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/editor/src/core/types/mention.ts(2 hunks)space/core/components/editor/lite-text-read-only-editor.tsx(2 hunks)space/core/components/editor/rich-text-editor.tsx(1 hunks)space/core/components/editor/rich-text-read-only-editor.tsx(2 hunks)web/core/components/editor/lite-text-editor/lite-text-editor.tsx(3 hunks)web/core/components/editor/lite-text-editor/lite-text-read-only-editor.tsx(3 hunks)web/core/components/editor/rich-text-editor/rich-text-editor.tsx(3 hunks)web/core/components/editor/rich-text-editor/rich-text-read-only-editor.tsx(3 hunks)web/core/components/pages/editor/editor-body.tsx(3 hunks)web/core/components/pages/version/editor.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- web/core/components/pages/editor/editor-body.tsx
- packages/editor/src/core/types/mention.ts
🧰 Additional context used
🧬 Code Definitions (1)
space/core/components/editor/rich-text-editor.tsx (4)
packages/editor/src/core/types/editor.ts (2) (2)
IRichTextEditor(133-137)EditorRefApi(91-108)web/core/components/editor/rich-text-editor/rich-text-editor.tsx (1) (1)
RichTextEditor(28-66)packages/editor/src/core/components/editors/rich-text/editor.tsx (1) (1)
RichTextEditorWithRef(45-45)space/core/components/editor/embeds/mentions/root.tsx (1) (1)
EditorMentionsRoot(8-17)
🔇 Additional comments (21)
web/core/components/editor/lite-text-editor/lite-text-read-only-editor.tsx (3)
11-12: Added required hook for user details retrieval.The
useMemberhook is now properly imported to access user information in the editor component.
24-25: Added user details retrieval capability.The
getUserDetailsfunction is extracted from theuseMemberhook, enabling the component to access user display names.
40-40: Fixed mention text copying issue.This change fixes the core issue by returning only the user's display name when a mention is processed, ensuring that when mentions are copied, they appear as text (
@{userDisplayName}) instead of HTML.web/core/components/editor/lite-text-editor/lite-text-editor.tsx (3)
16-17: Added required hook for user details retrieval.The
useMemberhook is now properly imported to access user information in the editor component.
62-63: Added user details retrieval capability.The
getUserDetailsfunction is extracted from theuseMemberhook, enabling the component to access user display names.
105-105: Fixed mention text copying issue.This change fixes the core issue by returning only the user's display name when a mention is processed, ensuring that when mentions are copied, they appear as text (
@{userDisplayName}) instead of HTML.space/core/components/editor/rich-text-editor.tsx (2)
8-9: Added required hook for user details retrieval.The
useMemberhook is now properly imported to access user information in the editor component.
17-17:❓ Verification inconclusive
Inconsistent implementation across components.
This component uses
getMemberByIdwhile other components usegetUserDetails. Also, the property accessed ismember__display_nameinstead ofdisplay_nameas in other components.- const { getMemberById } = useMember(); + const { getUserDetails } = useMember();
🏁 Script executed:
#!/bin/bash # Check implementation of useMember hook to see if getUserDetails and getMemberById are both valid methods rg -t typescript "export const useMember" -A 10Length of output: 84
Please run the following revised shell script to gather the necessary details on the implementation of the
useMemberhook. The previous command used an unrecognized file type ("typescript"), so this script searches both TSX and TS files instead:Once we have this output, we can determine whether the hook exports both
getMemberByIdandgetUserDetailsto resolve the inconsistency noted in the review comment.
🏁 Script executed:
#!/bin/bash echo "Searching for 'useMember' export in TSX files:" rg -t tsx "export const useMember" -A 30 || true echo "Searching for 'useMember' export in TS files:" rg -t ts "export const useMember" -A 30 || trueLength of output: 4090
Please run the following revised shell script to search for occurrences of both
getMemberByIdandgetUserDetailsacross all TS and TSX files. This will help us verify which method(s) are available in the codebase and confirm the inconsistency noted in the review comment.After you run this, please share the output so we can confirm if the inconsistency still exists and whether the suggested change (switching from
getMemberByIdtogetUserDetails) is indeed required.
🏁 Script executed:
#!/bin/bash echo "Searching for 'getMemberById' occurrences in TS/TSX files:" rg -g '*.{ts,tsx}' "getMemberById" || true echo "Searching for 'getUserDetails' occurrences in TS/TSX files:" rg -g '*.{ts,tsx}' "getUserDetails" || trueLength of output: 17243
Action Required: Verify & Unify Member Detail API Implementation
There’s an inconsistency in how member details are being retrieved between components. In the rich-text editor (in the space module) you’re currently using
const { getMemberById } = useMember();which returns a member object whose display name is accessed via
member__display_name. In contrast, most other components (in the web module) useconst { getUserDetails } = useMember();accessing the display name with the standard
display_nameproperty.Please verify the following:
- Is the dual API (i.e.
getMemberByIdvs.getUserDetails) an intentional separation between the space and web implementations, or should the APIs be unified?- If unification is desired, update the space store (and the corresponding
useMemberhook) to exportgetUserDetailsso that the rich-text editor can use the standardized member details.Affected file:
space/core/components/editor/rich-text-editor.tsx(line 17)Suggested update (if unification is the goal):
- const { getMemberById } = useMember(); - // currently uses: getMemberById(id)?.member__display_name + const { getUserDetails } = useMember(); + // then use: getUserDetails(id)?.display_namePlease review the underlying store logic to ensure that updating to
getUserDetailswon’t introduce regressions in the space module.web/core/components/editor/rich-text-editor/rich-text-editor.tsx (3)
16-17: Added required hook for user details retrieval.The
useMemberhook is now properly imported to access user information in the editor component.
31-32: Added user details retrieval capability.The
getUserDetailsfunction is extracted from theuseMemberhook, enabling the component to access user display names.
60-60: Fixed mention text copying issue.This change fixes the core issue by returning only the user's display name when a mention is processed, ensuring that when mentions are copied, they appear as text (
@{userDisplayName}) instead of HTML.web/core/components/pages/version/editor.tsx (3)
15-17: LGTM: Properly importing the member hook.The
useMemberhook import is appropriately added to retrieve user information needed for mention text handling.
30-31: LGTM: Clean implementation of member hook.The hook is properly integrated with a well-organized code structure and clear comment.
108-108: LGTM: Good implementation for mentions text copying.This implementation addresses the PR objective by retrieving the user's display name for mentions, ensuring that when copied, mentions will be represented as
@{userDisplayName}instead of copying the entire HTML node. The null coalescing operator provides appropriate fallback handling.web/core/components/editor/rich-text-editor/rich-text-read-only-editor.tsx (3)
9-10: LGTM: Appropriate hook import.The import of the member hook is correctly added to support the mention text handling functionality.
24-26: LGTM: Consistent implementation of member hook.The hook implementation follows the same pattern as in other components, maintaining code consistency.
40-40: LGTM: Proper mention text handling implementation.This implementation addresses the PR objective by retrieving the user's display name for mentions, ensuring that when copied, mentions will be represented as
@{userDisplayName}instead of copying the entire HTML node.space/core/components/editor/rich-text-read-only-editor.tsx (2)
9-10: LGTM: Appropriate hook import.The import of the member hook is correctly added to support the mention text handling functionality.
20-40: LGTM: Consistent implementation with proper member retrieval.The implementation properly retrieves member information using
getMemberByIdand accesses the correct propertymember__display_name, which differs slightly from the web implementation but is appropriate for the space context. This ensures that when mentions are copied, they will be represented as@{userDisplayName}.space/core/components/editor/lite-text-read-only-editor.tsx (2)
9-10: LGTM: Appropriate hook import.The import of the member hook is correctly added to support the mention text handling functionality.
20-40: LGTM: Consistent implementation with proper member retrieval.The implementation follows the same pattern as the rich text editor in the space context, correctly using
getMemberByIdandmember__display_name. This ensures consistency across components while properly addressing the mention text copying issue.
| getMentionedEntityDetails: (id: string) => ({ | ||
| display_name: getMemberById(id)?.member__display_name ?? "", | ||
| }), |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fixed mention text copying issue with inconsistent property naming.
This change fixes the core issue by returning only the user's display name when a mention is processed, but uses a different property name (member__display_name) compared to other components (display_name).
- getMentionedEntityDetails: (id: string) => ({
- display_name: getMemberById(id)?.member__display_name ?? "",
- }),
+ getMentionedEntityDetails: (id: string) => ({
+ display_name: getUserDetails(id)?.display_name ?? ""
+ }),🏁 Script executed:
#!/bin/bash
# Check both implementations to confirm the correct property access
rg -t typescript "getUserDetails.*\.display_name"
rg -t typescript "getMemberById.*\.member__display_name"Length of output: 180
Additional Verification Required: Recheck Property Access in Mentioned Entity Details
The initial rg -t typescript commands resulted in an error due to an unrecognized file type. To verify that the change correctly replaces the access from
getMemberById(id)?.member__display_name
to
getUserDetails(id)?.display_name
across the codebase, please run the following shell script that targets .tsx files directly:
Once verified, confirm that there are no unintended usages of the deprecated property access.
🏁 Script executed:
#!/bin/bash
# Re-run search queries with an explicit glob for .tsx files
echo "Searching for getUserDetails with .display_name in .tsx files..."
rg "getUserDetails.*\.display_name" --glob "*.tsx"
echo "Searching for getMemberById with .member__display_name in .tsx files..."
rg "getMemberById.*\.member__display_name" --glob "*.tsx"Length of output: 5326
Fix Inconsistent Mentioned Entity Property Across Space Modules
The recent update correctly changes the implementation in several components (e.g., in the web module) to use getUserDetails(id)?.display_name instead of getMemberById(id)?.member__display_name. However, our recent search reveals that in the space modules the old accessor is still in use. In particular:
- space/core/components/editor/rich-text-editor.tsx still uses:
getMentionedEntityDetails: (id: string) => ({ display_name: getMemberById(id)?.member__display_name ?? "", }),
- space/core/components/editor/rich-text-read-only-editor.tsx and
space/core/components/editor/lite-text-read-only-editor.tsx exhibit the same outdated approach.
Please update these files to use:
- getMentionedEntityDetails: (id: string) => ({
- display_name: getMemberById(id)?.member__display_name ?? "",
- }),
+ getMentionedEntityDetails: (id: string) => ({
+ display_name: getUserDetails(id)?.display_name ?? "",
+ }),This change will ensure consistent property naming for mention processing across all modules.
Description
Before: The whole html of the mention node got copied
Now: The renderText method calculates and returns the correct the text properly as
@{userDisplayName}Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit