[WIKI-345] chore: pass disabled and flagged extensions to block menu#7152
[WIKI-345] chore: pass disabled and flagged extensions to block menu#7152sriramveeraghanta merged 32 commits intopreviewfrom
Conversation
WalkthroughThis update introduces support for a new "external-embed" editor extension, propagates a Changes
Sequence Diagram(s)sequenceDiagram
participant EditorContainer
participant CollaborativeEditor
participant ReadOnlyEditor
participant PageRenderer
participant BlockMenu
EditorContainer->>CollaborativeEditor: Pass disabledExtensions
EditorContainer->>ReadOnlyEditor: Pass disabledExtensions
CollaborativeEditor->>PageRenderer: Pass disabledExtensions
ReadOnlyEditor->>PageRenderer: Pass disabledExtensions
PageRenderer->>BlockMenu: Pass disabledExtensions
sequenceDiagram
participant Editor
participant getExtensionStorage
participant CustomLinkStorage
Editor->>getExtensionStorage: Request storage for CORE_EXTENSIONS.CUSTOM_LINK
getExtensionStorage-->>Editor: Return storage (may be undefined)
Editor->>CustomLinkStorage: Access isBubbleMenuOpen (safely)
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
🧹 Nitpick comments (3)
live/src/core/services/api.service.ts (1)
17-17: Good addition of request timeout, consider documenting the value choice.Adding a timeout to prevent hanging requests is a solid improvement for the API service. The 20-second timeout is reasonable for most API operations.
Consider adding a comment explaining why 20 seconds was chosen, or potentially making this configurable via environment variables for different deployment scenarios.
+ // 20 second timeout to prevent hanging requests timeout: 20000,web/ce/components/pages/editor/external-embed/embed-handler.tsx (1)
1-3: Add TODO comment for placeholder implementation.The
EmbedHandlercomponent is currently a placeholder that acceptsNodeViewPropsbut renders an empty<div>. Consider adding a TODO comment to clarify this is intentional and needs future implementation.+// TODO: Implement embed handling logic export const EmbedHandler: React.FC<NodeViewProps> = () => <div />;packages/editor/src/core/components/menus/block-menu.tsx (1)
11-11: Consider using the disabledExtensions prop or add TODO.The
disabledExtensionsprop is added to the interface but not used in the component implementation. Either implement the logic to handle disabled extensions or add a TODO comment indicating future implementation plans.interface BlockMenuProps { editor: Editor; + // TODO: Implement logic to handle disabled extensions in menu items disabledExtensions?: TExtensions[]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
live/src/core/services/api.service.ts(1 hunks)packages/editor/src/core/components/editors/document/collaborative-editor.tsx(1 hunks)packages/editor/src/core/components/editors/document/page-renderer.tsx(3 hunks)packages/editor/src/core/components/editors/document/read-only-editor.tsx(1 hunks)packages/editor/src/core/components/editors/editor-container.tsx(1 hunks)packages/editor/src/core/components/editors/link-view-container.tsx(4 hunks)packages/editor/src/core/components/menus/block-menu.tsx(1 hunks)packages/editor/src/core/extensions/custom-link/extension.tsx(1 hunks)packages/editor/src/core/plugins/drag-handle.ts(1 hunks)packages/editor/src/core/types/editor.ts(3 hunks)packages/editor/src/core/types/extensions.ts(1 hunks)packages/editor/src/core/types/hook.ts(2 hunks)packages/editor/src/styles/drag-drop.css(2 hunks)space/ce/components/editor/external-embed/embed-handler.tsx(1 hunks)space/helpers/string.helper.ts(1 hunks)web/ce/components/pages/editor/external-embed/embed-handler.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (16)
packages/editor/src/core/extensions/custom-link/extension.tsx (2)
82-82: LGTM: Clean addition of bubble menu state tracking.The new
isBubbleMenuOpenproperty follows consistent naming conventions and provides proper type safety for bubble menu state management.
258-258: LGTM: Proper initialization of new state property.The initialization of
isBubbleMenuOpentofalseis appropriate and maintains consistency with the existing storage pattern.packages/editor/src/core/components/editors/editor-container.tsx (1)
6-6: LGTM: Import path standardization.The change from relative to absolute alias path improves consistency and maintainability of import statements across the codebase.
packages/editor/src/core/components/editors/document/collaborative-editor.tsx (1)
98-98: LGTM: Proper prop forwarding for extension control.The
disabledExtensionsprop is correctly passed down toPageRenderer, maintaining the prop propagation chain for extension awareness across editor components.packages/editor/src/core/components/menus/block-menu.tsx (1)
7-7: LGTM: Proper type import addition.The import of
TExtensionstype supports the newdisabledExtensionsprop and follows proper import organization.packages/editor/src/core/plugins/drag-handle.ts (1)
106-109: LGTM! Consistent filtering logic for embed elements.The new filtering condition correctly prevents selection of child elements within
.data-embedcontainers while allowing selection of the.data-embedelements themselves. This follows the same pattern as the existing table wrapper filtering and maintains consistent drag handle behavior.packages/editor/src/core/types/extensions.ts (1)
1-8: LGTM! Clean type extension for external embed support.The addition of
"external-embed"to theTExtensionstype union correctly enables type safety for the new extension throughout the codebase.packages/editor/src/core/components/editors/document/read-only-editor.tsx (1)
67-67: LGTM! Consistent prop forwarding for extension management.The addition of
disabledExtensions={disabledExtensions}prop forwarding toPageRenderermaintains consistency with similar changes in other editor components and enables proper extension management throughout the component hierarchy.packages/editor/src/core/components/editors/link-view-container.tsx (2)
23-23: LGTM! Safer extension storage access.Replacing direct
editor.storage.linkaccess withgetExtensionStorage(editor, CORE_EXTENSIONS.CUSTOM_LINK)utility function provides safer access to extension storage and follows defensive programming practices.
52-52: LGTM! Proper null safety with optional chaining.The addition of optional chaining (
?.) when accessingisBubbleMenuOpenprevents potential runtime errors if the extension storage is undefined, improving the robustness of the component.Also applies to: 115-115
packages/editor/src/core/types/hook.ts (1)
26-26: LGTM! Consistent type extension for embed support.The addition of
"embedHandler"to bothTEditorHookPropsandTReadOnlyEditorHookPropsmaintains consistency with the editor interface changes and follows the existing pattern of picking properties from the respective editor interfaces.Also applies to: 51-51
packages/editor/src/core/components/editors/document/page-renderer.tsx (1)
6-6: LGTM! Proper prop propagation for disabled extensions.The implementation correctly:
- Imports the required
TExtensionstype- Adds the
disabledExtensionsprop to the component interface- Destructures the prop in the component
- Passes it down to the
BlockMenucomponentThis follows React best practices for prop drilling and maintains type safety.
Also applies to: 16-16, 28-28, 43-43
space/helpers/string.helper.ts (1)
74-80: LGTM! Logical extension for embed content support.The addition of
"embed-component"and"div[data-embed]"to the allowed HTML tags appropriately extends the function to recognize embed content as valid non-empty content. The changes maintain backward compatibility while supporting the new embed functionality.packages/editor/src/core/types/editor.ts (2)
49-51: LGTM! Proper extension of editor commands.The addition of
"external-embed"and"rich-card"to theTEditorCommandsunion type correctly extends the available editor commands while maintaining the existing structure.
132-132: LGTM! Consistent embedHandler property integration.The
embedHandlerproperty is properly added as optional toIEditorPropsand correctly included in the picked properties forIReadOnlyEditorProps. This maintains consistency across the editor interface hierarchy.Also applies to: 163-163
packages/editor/src/styles/drag-drop.css (1)
39-39: LGTM! Consistent CSS integration for external embed components.The addition of
.node-externalEmbedComponentto both the exclusion selector (line 39) and the specific styling rules (line 66) maintains visual consistency with existing image node components. This ensures that external embed components receive the same drag-and-drop styling treatment as other media content.Also applies to: 66-66
| editorContainerClassName: string; | ||
| id: string; | ||
| tabIndex?: number; | ||
| disabledExtensions: TExtensions[]; |
There was a problem hiding this comment.
Extend this from IEditorProps
|
|
||
| interface BlockMenuProps { | ||
| editor: Editor; | ||
| disabledExtensions?: TExtensions[]; |
There was a problem hiding this comment.
Extend the type here as well.
| } | ||
|
|
||
| // Skip elements inside .data-embed | ||
| if (elem.closest(".data-embed") && !elem.matches(".data-embed")) { |
| id: string; | ||
| initialValue: string; | ||
| mentionHandler: TMentionHandler; | ||
| embedHandler?: TEmbedConfig; |
There was a problem hiding this comment.
Update the types to define the embedHandler only once and extend for all other use cases.
| interface EmbedHandlerProps extends NodeViewProps { | ||
| anchor?: string; | ||
| } | ||
| export const EmbedHandler: React.FC<EmbedHandlerProps> = () => <div />; |
There was a problem hiding this comment.
This component should return null.
space/helpers/string.helper.ts
Outdated
| "mention-component", | ||
| "image-component", | ||
| "embed-component", | ||
| `div[data-embed]`, |
There was a problem hiding this comment.
This is not needed, embed-component should be enough.
| @@ -0,0 +1,3 @@ | |||
| import { NodeViewProps } from "@tiptap/react"; | |||
|
|
|||
| export const EmbedHandler: React.FC<NodeViewProps> = () => <div />; | |||
There was a problem hiding this comment.
Make this component return null.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
packages/editor/src/core/extensions/extensions.ts (1)
44-50: Consider removing redundant embedHandler declaration.The
embedHandlerappears to be declared twice - once in thePicktype (line 50) and once in the extended object (line 54). This redundancy could be confusing.Consider this approach to avoid duplication:
type TArguments = Pick< IEditorProps, | "disabledExtensions" | "flaggedExtensions" | "fileHandler" | "mentionHandler" | "placeholder" | "tabIndex" | "embedHandler" > & { enableHistory: boolean; editable: boolean; - embedHandler?: TEmbedConfig; };The
embedHandlerfromIEditorPropsshould already be optional, making the second declaration unnecessary.Also applies to: 54-54
space/core/components/editor/rich-text-editor.tsx (1)
44-48: Consider extracting embed handler configuration for performance.The inline object creation will cause the
embedHandlerprop to have a new reference on every render, potentially causing unnecessary re-renders of the underlying editor component.Consider extracting the configuration using
useMemo:+import React, { forwardRef, useMemo } from "react"; export const RichTextEditor = forwardRef<EditorRefApi, RichTextEditorWrapperProps>((props, ref) => { const { anchor, containerClassName, uploadFile, workspaceId, disabledExtensions, flaggedExtensions, ...rest } = props; const { getMemberById } = useMember(); + + const embedHandler = useMemo( + () => ({ + externalEmbedComponent: { + widgetCallback: (props: NodeViewProps) => <EmbedHandler anchor={anchor} {...props} />, + }, + }), + [anchor] + ); return ( <RichTextEditorWithRef // ... other props - embedHandler={{ - externalEmbedComponent: { - widgetCallback: (props: NodeViewProps) => <EmbedHandler anchor={anchor} {...props} />, - }, - }} + embedHandler={embedHandler} // ... rest of props /> ); });space/core/components/editor/lite-text-read-only-editor.tsx (1)
26-28: Consider memoizing embed handler configuration.The
embedHandlerConfigis recreated on every render, which could cause unnecessary re-renders of the underlying editor component.Apply this optimization using
useMemo:-import React from "react"; +import React, { useMemo } from "react"; export const LiteTextReadOnlyEditor = React.forwardRef<EditorReadOnlyRefApi, LiteTextReadOnlyEditorWrapperProps>( ({ anchor, workspaceId, disabledExtensions, flaggedExtensions, ...props }, ref) => { const { getMemberById } = useMember(); - const embedHandlerConfig = { - externalEmbedComponent: { widgetCallback: (props: NodeViewProps) => <EmbedHandler {...props} anchor={anchor} /> }, - }; + + const embedHandlerConfig = useMemo( + () => ({ + externalEmbedComponent: { widgetCallback: (props: NodeViewProps) => <EmbedHandler {...props} anchor={anchor} /> }, + }), + [anchor] + );space/core/components/editor/rich-text-read-only-editor.tsx (1)
43-47: Consider memoizing embed handler for performance consistency.Similar to the other components, the inline object creation causes a new reference on every render.
For consistency with the suggested optimization in other components:
+import React, { useMemo } from "react"; export const RichTextReadOnlyEditor = React.forwardRef<EditorReadOnlyRefApi, RichTextReadOnlyEditorWrapperProps>( ({ anchor, workspaceId, disabledExtensions, flaggedExtensions, ...props }, ref) => { const { getMemberById } = useMember(); + + const embedHandler = useMemo( + () => ({ + externalEmbedComponent: { + widgetCallback: (props: NodeViewProps) => <EmbedHandler anchor={anchor} {...props} />, + }, + }), + [anchor] + ); return ( <RichTextReadOnlyEditorWithRef // ... other props - embedHandler={{ - externalEmbedComponent: { - widgetCallback: (props: NodeViewProps) => <EmbedHandler anchor={anchor} {...props} />, - }, - }} + embedHandler={embedHandler} // ... rest of props /> ); } );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/editor/src/ce/extensions/core/extensions.ts(1 hunks)packages/editor/src/ce/types/issue-embed.ts(1 hunks)packages/editor/src/core/components/editors/editor-container.tsx(2 hunks)packages/editor/src/core/components/editors/editor-wrapper.tsx(2 hunks)packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx(2 hunks)packages/editor/src/core/extensions/extensions.ts(3 hunks)packages/editor/src/core/extensions/read-only-extensions.ts(1 hunks)packages/editor/src/core/hooks/use-collaborative-editor.ts(1 hunks)packages/editor/src/core/hooks/use-editor.ts(2 hunks)packages/editor/src/core/hooks/use-read-only-editor.ts(2 hunks)packages/editor/src/core/plugins/drag-handle.ts(2 hunks)packages/editor/src/core/types/editor.ts(3 hunks)space/core/components/editor/lite-text-editor.tsx(4 hunks)space/core/components/editor/lite-text-read-only-editor.tsx(4 hunks)space/core/components/editor/rich-text-editor.tsx(3 hunks)space/core/components/editor/rich-text-read-only-editor.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/editor/src/core/components/editors/editor-container.tsx
- packages/editor/src/core/plugins/drag-handle.ts
- packages/editor/src/core/types/editor.ts
🔇 Additional comments (23)
packages/editor/src/core/components/editors/read-only-editor-wrapper.tsx (1)
25-25: Clean prop propagation for embed handler support.The addition of
embedHandlerprop follows the established pattern used by other handler props likementionHandlerandfileHandler. The implementation is clean and maintains backward compatibility.Also applies to: 37-37
packages/editor/src/core/hooks/use-read-only-editor.ts (1)
28-28: Consistent integration of embed handler in read-only editor hook.The
embedHandlerprop is properly integrated following the same pattern as existing handler props (mentionHandler,fileHandler). The implementation maintains consistency with the hook's established architecture.Also applies to: 53-53
packages/editor/src/ce/extensions/core/extensions.ts (1)
7-7: Type-safe extension of core additional extensions props.Adding
"embedHandler"to theTCoreAdditionalExtensionsPropstype ensures proper TypeScript support for the new prop while maintaining type safety throughout the extension system.packages/editor/src/core/hooks/use-collaborative-editor.ts (1)
78-78: Completes embed handler integration in collaborative editor.This change fixes the missing
embedHandlerprop forwarding touseEditor. The prop was already being destructured from the function parameters but wasn't being passed down, which this change properly addresses.packages/editor/src/core/components/editors/editor-wrapper.tsx (1)
39-39: Consistent embed handler support across editor variants.The
embedHandlerprop integration follows the same clean pattern established in other editor components, ensuring feature parity between editable and read-only editor variants.Also applies to: 61-61
packages/editor/src/core/hooks/use-editor.ts (2)
39-39: LGTM: embedHandler prop added consistently.The
embedHandlerprop is properly destructured and follows the same pattern as other handler props in the hook.
70-70: LGTM: embedHandler properly forwarded to extensions.The
embedHandleris correctly passed toCoreEditorExtensions, maintaining consistency with the existing prop forwarding pattern.packages/editor/src/ce/types/issue-embed.ts (2)
1-2: LGTM: Proper import for NodeViewProps.The import is correctly added to support the new type definition.
5-5: LGTM: Well-structured type definitions for external embed support.The
TExternalEmbedConfigtype properly defines the interface for external embed components with awidgetCallbackthat follows Tiptap's NodeView pattern. The optional nature ofexternalEmbedComponentinTEmbedConfigis appropriate.Also applies to: 8-9
packages/editor/src/core/extensions/extensions.ts (2)
40-40: LGTM: Proper import for TEmbedConfig type.The import correctly adds the
TEmbedConfigtype needed for the embedHandler functionality.
66-66: LGTM: Proper implementation of embedHandler forwarding.The
embedHandleris correctly destructured and passed toCoreEditorAdditionalExtensions, following the established pattern for other handler props.Also applies to: 196-197
space/core/components/editor/lite-text-editor.tsx (3)
3-3: LGTM: Proper imports for embed functionality.The imports for
NodeViewPropsandEmbedHandlerare correctly added to support the new embed functionality.Also applies to: 12-12
16-16: LGTM: Proper interface design.Correctly omitting
embedHandlerfrom the props interface since it's handled internally by the component. This maintains a clean API surface.
44-46: LGTM: Well-structured embed handler configuration.The
embedHandlerConfigis properly structured following theTExternalEmbedConfiginterface. ThewidgetCallbackcorrectly passes throughNodeViewPropsand includes theanchorprop for context. The configuration is appropriately passed to the editor component.Also applies to: 62-62
space/core/components/editor/rich-text-editor.tsx (3)
3-3: LGTM: Proper import added for type safety.The
NodeViewPropsimport is correctly added to support the embed handler widget callback typing.
12-12: LGTM: EmbedHandler component import looks good.The import path appears consistent with the overall architecture and the component is used appropriately in the embed handler configuration.
16-16: LGTM: Proper props interface refinement.Adding
embedHandlerto the omitted props list is correct since it's now handled internally by the wrapper component.space/core/components/editor/lite-text-read-only-editor.tsx (3)
3-3: LGTM: Consistent import pattern.The
NodeViewPropsimport is correctly added, maintaining consistency with other editor components.
13-13: LGTM: EmbedHandler import follows established pattern.The import is consistent with the other editor components in this refactoring.
16-16: LGTM: Props interface properly updated.Correctly omitting
embedHandlerfrom inherited props since it's handled internally.space/core/components/editor/rich-text-read-only-editor.tsx (3)
3-3: LGTM: Import consistency maintained.The
NodeViewPropsimport maintains consistency across all refactored editor components.
13-13: LGTM: Consistent EmbedHandler integration.The import follows the established pattern from the other editor components in this refactoring.
16-16: LGTM: Proper interface refinement.Correctly omitting
embedHandlerfrom inherited props maintains the established pattern.
| type Props = Pick< | ||
| IReadOnlyEditorProps, | ||
| "disabledExtensions" | "flaggedExtensions" | "fileHandler" | "mentionHandler" | "embedHandler" | ||
| >; |
There was a problem hiding this comment.
Type update looks correct but implementation is incomplete.
The Props type correctly includes embedHandler, but the parameter is not being destructured or used in the CoreReadOnlyEditorExtensions function.
🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/read-only-extensions.ts around lines 36
to 39, the Props type includes embedHandler but the CoreReadOnlyEditorExtensions
function does not destructure or use embedHandler from its parameters. Update
the function signature to destructure embedHandler from Props and ensure it is
properly utilized within the function implementation.
| >; | ||
|
|
||
| export const CoreReadOnlyEditorExtensions = (props: Props): Extensions => { | ||
| const { disabledExtensions, fileHandler, flaggedExtensions, mentionHandler } = props; |
There was a problem hiding this comment.
Add embedHandler to destructuring and pass it to extensions.
The embedHandler should be destructured from props and likely passed to CoreReadOnlyEditorAdditionalExtensions similar to how it's done in the main extensions file.
Apply this diff to fix the implementation:
- const { disabledExtensions, fileHandler, flaggedExtensions, mentionHandler } = props;
+ const { disabledExtensions, fileHandler, flaggedExtensions, mentionHandler, embedHandler } = props;And update the CoreReadOnlyEditorAdditionalExtensions call to include embedHandler:
...CoreReadOnlyEditorAdditionalExtensions({
disabledExtensions,
flaggedExtensions,
+ embedHandler,
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { disabledExtensions, fileHandler, flaggedExtensions, mentionHandler } = props; | |
| // Destructure embedHandler along with the other props | |
| const { disabledExtensions, fileHandler, flaggedExtensions, mentionHandler, embedHandler } = props; | |
| return createEditorExtensions([ | |
| // …other extensions… | |
| ...CoreReadOnlyEditorAdditionalExtensions({ | |
| disabledExtensions, | |
| flaggedExtensions, | |
| embedHandler, | |
| }), | |
| ]); |
🤖 Prompt for AI Agents
In packages/editor/src/core/extensions/read-only-extensions.ts at line 42, the
embedHandler is missing from the destructuring of props. Add embedHandler to the
destructuring assignment alongside disabledExtensions, fileHandler,
flaggedExtensions, and mentionHandler. Then, pass embedHandler as an argument to
the CoreReadOnlyEditorAdditionalExtensions call to ensure it is properly
included in the extensions.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/core/components/pages/version/editor.tsx (1)
3-3: LGTM! Consistent implementation of external embed support.The external embed handler integration follows a consistent pattern across the codebase. The imports and configuration are properly implemented.
Consider creating a shared utility function to reduce code duplication across editor components:
// utils/editor-embed-handler.ts import { NodeViewProps } from "@tiptap/react"; import { EmbedHandler } from "@/plane-web/components/pages/editor/external-embed/embed-handler"; export const createEmbedHandlerConfig = () => ({ externalEmbedComponent: { widgetCallback: (props: NodeViewProps) => <EmbedHandler {...props} /> }, });This could be reused across all editor components to maintain consistency and reduce duplication.
Also applies to: 17-17, 121-121
web/core/components/editor/rich-text-editor/rich-text-editor.tsx (1)
3-3: LGTM! Completes consistent external embed integration across all editor types.The rich text editor properly integrates the external embed handler, maintaining consistency with the refactoring pattern applied across all editor components. The embedHandler is correctly omitted from the wrapper props and configured appropriately.
This completes the external embed integration across all editor variants. Consider the shared utility function suggestion from the first file to reduce the code duplication across all these editor components.
Also applies to: 16-16, 20-20, 72-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
web/core/components/editor/lite-text-editor/lite-text-editor.tsx(4 hunks)web/core/components/editor/lite-text-editor/lite-text-read-only-editor.tsx(4 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(2 hunks)web/core/components/pages/version/editor.tsx(3 hunks)web/next.config.js(1 hunks)
🔇 Additional comments (4)
web/next.config.js (1)
15-15: Good security hardening, but verify no breaking changes.Changing X-Frame-Options from "SAMEORIGIN" to "DENY" strengthens security by preventing clickjacking attacks. This complements the editor embedding enhancements mentioned in the PR summary by allowing internal content embedding while preventing external framing of the entire application.
However, ensure this doesn't break any existing functionality where the application might have been embedded in same-origin iframes.
#!/bin/bash # Description: Search for potential iframe usage or embedding references in the codebase # Expected: Find any references to iframe embedding or frame-related functionality echo "Searching for iframe usage and frame-related code..." rg -i "iframe|frame|embed" --type js --type jsx --type ts --type tsx -A 2 -B 2 echo -e "\nSearching for X-Frame-Options references..." rg -i "x-frame-options|frameOptions" -A 2 -B 2 echo -e "\nSearching for embedding-related documentation..." fd -e md | xargs rg -i "embed|iframe|frame" -A 2 -B 2web/core/components/editor/rich-text-editor/rich-text-read-only-editor.tsx (1)
15-15: LGTM! Proper integration of embed handler in read-only editor.The external embed handler is correctly integrated following the established pattern. The implementation maintains consistency with other editor components.
Also applies to: 18-18, 54-54
web/core/components/editor/lite-text-editor/lite-text-editor.tsx (1)
3-3: LGTM! Clean implementation with separate config object.The implementation properly integrates the external embed handler. The approach of creating a separate
embedHandlerConfigobject improves readability compared to inline configuration.Also applies to: 22-22, 27-27, 89-91, 120-120
web/core/components/editor/lite-text-editor/lite-text-read-only-editor.tsx (1)
3-3: LGTM! Consistent implementation with clean config object pattern.The read-only lite text editor properly integrates the external embed handler using the same clean config object approach as its editable counterpart. Good consistency across related components.
Also applies to: 16-16, 19-19, 36-38, 54-54
| @@ -0,0 +1,6 @@ | |||
| import { NodeViewProps } from "@tiptap/react"; | |||
There was a problem hiding this comment.
please import it as { type NodeViewProps }
iam-vipin
changed the title
iam-vipin
changed the title
4 participants
Description
This PR pass disabled and flagged extension in block menu in editor
Type of Change
Screenshots and Media (if applicable)