Merge GrapesJS OSS plugins into WYSIWYG plugin code #13
Conversation
🦋 Changeset detectedLatest commit: 5294b2c The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
""" WalkthroughThis update merges multiple open-source GrapesJS plugins directly into the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor (WYSIWYG)
participant LocalPlugin
participant Modal/Component
User->>Editor (WYSIWYG): Opens editor
Editor (WYSIWYG)->>LocalPlugin: Initializes local plugins (blocks, forms, code, etc.)
LocalPlugin-->>Editor (WYSIWYG): Registers components, blocks, traits
User->>Editor (WYSIWYG): Interacts (e.g., adds block, edits code)
Editor (WYSIWYG)->>Modal/Component: Opens modal/component for editing (e.g., code, image)
Modal/Component-->>Editor (WYSIWYG): Applies changes
Editor (WYSIWYG)-->>User: Updates canvas/UI
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 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 (
|
commit: |
Adammatthiesen
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🧹 Nitpick comments (41)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/styles.css (4)
5-10: Consider making the toolbar action buttons more visually distinct.While
.gjs-rte-actionsets minimal styling like font-size and padding, giving these buttons a hover state or a more prominent border/fill could improve the user experience.
20-29: Reduce the 2-second transition for better responsiveness.The line
transition: all 2s ease;makes the color picker animations feel sluggish. Shortening the duration (e.g.,0.2s) can provide a smoother user experience without long delays.- transition: all 2s ease; + transition: all 0.2s ease;
30-37: Validate arrow positioning for different screen sizes.The arrow (
.rte-color-picker:before) is positioned atleft: 46%;. This might not center correctly on narrower or responsive layouts. Consider a dynamic calculation or transform-based approach for consistent arrow centering.
61-63: Ensure naming consistency and reusability.
.picker-wrapperis generic. If used only for this color picker, consider naming it more specifically (e.g.,.rte-picker-wrapper) to avoid possible naming collisions with other modules or global styles.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts (4)
12-16: Fix duplicate doc comment.
idandlabelTooltipboth have similar doc strings: "The ID used to create tooltip block and component." Update the description forlabelTooltipto avoid confusion.- /** - * The ID used to create tooltip block and component - * @default 'Tooltip' - */ + /** + * The label used to identify and display the tooltip in the UI + * @default 'Tooltip' + */
81-144: Handle missing or invalidoptsmore robustly.Currently, the plugin merges default options with
optsbut does not validate keys likeid,labelTooltip, orclassTooltip. Invalid or null values could break the component. Consider adding a light validation or fallback logic.
186-192: Check for potential collisions with private classes.Marking classes as private with
private: 1ensures user styling won't interfere. However, if there's a scenario where the user wants partial styling control (for theme customization), consider making some classes partially adjustable instead of fully private.
315-343: Allow more dynamic or user-friendly tooltips.Currently, the text is static, entered in 'Text' trait. For advanced use cases, consider a toggle or dynamic data binding for the tooltip content.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/colorPicker.ts (2)
49-74: Document the default color set or make it user-configurable.The large default color palette is helpful, but clearly documenting the default options in the README or usage docs (beyond code comments) would assist users in customizing their color sets.
95-103: Re-check initialisOpenstate logic.Declaring
this.isOpen = true;(line 95) before callingclose()at line 129 might cause a brief flicker in some browsers or frameworks. If the intent is to start closed, consider settingthis.isOpen = falseinitially.- this.isOpen = true; + this.isOpen = false;packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/blocks.ts (1)
4-23: Functional tabs block implementation with minor style inconsistencyThe tabs block implementation works correctly, conditionally adding the block based on configuration. I noticed this file uses a slightly different coding style compared to the other block files (named function, explicit return type, local blockConfig variable).
Consider standardizing the coding style across plugin files for better maintainability:
-export default function loadBlocks( - editor: Editor, - { tabsBlock, typeTabs }: Pick<TabsOptions, 'tabsBlock' | 'typeTabs'> -): void { +export default (editor: Editor, { tabsBlock, typeTabs }: Pick<TabsOptions, 'tabsBlock' | 'typeTabs'>) => {packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContent.ts (1)
1-24: Well-structured component with good accessibility supportThe TabContent component is well-implemented with appropriate accessibility attributes (role="tabpanel") and sensible restrictions for the editor interface. The configuration is flexible through the passed config object.
Consider adding JSDoc comments to document the purpose of this component and its relationship with other tab components. Also, the exported
roleconstant could potentially conflict with other imports if multiple files export the same name - consider a more specific name liketabContentRole.-export const role = 'tabpanel'; +/** ARIA role for tab content panels */ +export const tabContentRole = 'tabpanel'; -export default (editor: Editor, config: TabContentConfig): void => { +/** + * Registers the Tab Content component type with the GrapesJS editor + * @param editor - The GrapesJS editor instance + * @param config - Configuration for the tab content component + */ +export default (editor: Editor, config: TabContentConfig): void => {And update the attribute reference:
- attributes: { role }, + attributes: { role: tabContentRole },packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContents.ts (1)
1-20: Consider adding ARIA role for accessibilityThe TabContents component looks well-structured, but unlike TabContent, it doesn't specify an ARIA role. Depending on its purpose in the DOM structure, it might benefit from an appropriate role like
regionorgroupto improve accessibility.Also, consider adding JSDoc documentation and ensuring consistency with the TabContent component in terms of properties like
highlightable(present in TabContent but missing here).+/** + * Registers the Tab Contents component type with the GrapesJS editor + * @param editor - The GrapesJS editor instance + * @param config - Configuration for the tab contents component + */ export default (editor: Editor, config: TabContentsConfig): void => { const { Components } = editor; Components.addType(config.typeTabContents, { model: { defaults: { name: 'Tab Contents', draggable: false, droppable: false, copyable: false, removable: false, + highlightable: false, + attributes: { role: 'region' }, classes: config.classTabContents, ...config.tabContentsProps, },packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/traits.ts (1)
4-5: Add JSDoc to the main functionAdd documentation to the main function to clarify its purpose and usage.
+/** + * Registers the 'select-options' trait type with the GrapesJS TraitManager + * This trait provides a textarea interface for editing select element options + * @param editor - The GrapesJS editor instance + */ export default function (editor: Editor) {packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/utils.ts (1)
1-3: Add documentation to constantsThese constants are well-named and centralized which is good practice. Consider adding JSDoc comments to explain their purpose and usage within the custom code plugin.
+/** Storage key for custom code content */ export const keyCustomCode = 'custom-code-plugin__code'; +/** Component type identifier for custom code blocks */ export const typeCustomCode = 'custom-code'; +/** Command name to open the custom code editing modal */ export const commandNameCustomCode = 'custom-code:open-modal';packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/blocks.ts (2)
195-216: Consider adding RTL support for column layouts.The 30/70 split layout assumes left-to-right reading direction. For better internationalization support, consider making these layouts respect RTL direction settings.
You could adapt the layout based on a configuration option, for example:
toAdd('column3-7') && bm.add('column3-7', { ...commonBlockProps, label: opts.labelColumn37, media: `<svg viewBox="0 0 24 24"> <path fill="currentColor" d="M2 20h5V4H2v16Zm-1 0V4a1 1 0 0 1 1-1h5a1 1 0 0 1 1 1v16a1 1 0 0 1-1 1H2a1 1 0 0 1-1-1ZM10 20h12V4H10v16Zm-1 0V4a1 1 0 0 1 1-1h12a1 1 0 0 1 1 1v16a1 1 0 0 1-1 1H10a1 1 0 0 1-1-1Z"/> </svg>`, content: `<div ${attrsRow}> + ${opts.rtl ? + `<div ${attrsCell} style='${flexGrid ? 'flex-basis' : 'width'}: 70%;'></div> + <div ${attrsCell} style='${flexGrid ? 'flex-basis' : 'width'}: 30%;'></div>` : + `<div ${attrsCell} style='${flexGrid ? 'flex-basis' : 'width'}: 30%;'></div> + <div ${attrsCell} style='${flexGrid ? 'flex-basis' : 'width'}: 70%;'></div>` + } - <div ${attrsCell} style='${flexGrid ? 'flex-basis' : 'width'}: 30%;'></div> - <div ${attrsCell} style='${flexGrid ? 'flex-basis' : 'width'}: 70%;'></div> </div>
247-259: The image block could use a default placeholder.The image block doesn't include a default placeholder source, which may lead to empty images in the editor. Consider adding a placeholder URL.
toAdd('image') && bm.add('image', { ...commonBlockProps, activate: true, label: opts.labelImage, media: `<svg viewBox="0 0 24 24"> <path fill="currentColor" d="M21,3H3C2,3 1,4 1,5V19A2,2 0 0,0 3,21H21C22,21 23,20 23,19V5C23,4 22,3 21,3M5,17L8.5,12.5L11,15.5L14.5,11L19,17H5Z" /> </svg>`, content: { style: { color: 'black' }, type: 'image', + src: 'data:image/svg+xml;base64,PHN2ZyB3aWR0aD0iMTAwIiBoZWlnaHQ9IjEwMCIgeG1sbnM9Imh0dHA6Ly93d3cudzMub3JnLzIwMDAvc3ZnIj4KPHJlY3Qgd2lkdGg9IjEwMCIgaGVpZ2h0PSIxMDAiIGZpbGw9IiNlZWVlZWUiLz4KPHRleHQgeD0iNTAiIHk9IjUwIiBmb250LXNpemU9IjEyIiB0ZXh0LWFuY2hvcj0ibWlkZGxlIiBhbGlnbm1lbnQtYmFzZWxpbmU9Im1pZGRsZSIgZm9udC1mYW1pbHk9InNhbnMtc2VyaWYiIGZpbGw9IiM5OTk5OTkiPkltYWdlPC90ZXh0Pgo8L3N2Zz4=' }, });packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/index.ts (1)
63-68: Consider adding translation support for placeholder text.The hardcoded placeholder text for script elements could benefit from internationalization support, allowing for localized messages.
const plugin: Plugin<PluginOptions> = (editor, opts = {}) => { + // Get translations API from editor + const i18n = editor.I18n; const options: PluginOptions = { blockCustomCode: {}, propsCustomCode: {}, toolbarBtnCustomCode: {}, - placeholderScript: `<div style="pointer-events: none; padding: 10px;"> - <svg viewBox="0 0 24 24" style="height: 30px; vertical-align: middle;"> - <path d="M13 14h-2v-4h2m0 8h-2v-2h2M1 21h22L12 2 1 21z"></path> - </svg> - Custom code with <i><script></i> can't be rendered on the canvas - </div>`, + placeholderScript: (opts.placeholderScript || i18n.getModel()?.get('customCode.placeholderScript') || + `<div style="pointer-events: none; padding: 10px;"> + <svg viewBox="0 0 24 24" style="height: 30px; vertical-align: middle;"> + <path d="M13 14h-2v-4h2m0 8h-2v-2h2M1 21h22L12 2 1 21z"></path> + </svg> + Custom code with <i><script></i> can't be rendered on the canvas + </div>`), modalTitle: 'Insert your code', codeViewOptions: {}, buttonLabel: 'Save', commandCustomCode: {}, ...opts, };packages/studiocms_wysiwyg/src/wysiwyg/components/Editor.astro (1)
207-217: Consider using dynamic imports for TOAST UI dependencies.The TOAST UI Image Editor resources are loaded from CDN URLs, which could lead to network failures or version conflicts. Consider dynamically importing these resources or bundling them locally.
If CDN resources must be used, you could implement a fallback mechanism:
// Example fallback implementation function loadScript(url, fallbackUrl) { return new Promise((resolve, reject) => { const script = document.createElement('script'); script.src = url; script.onload = resolve; script.onerror = () => { console.warn(`Failed to load ${url}, trying fallback...`); const fallback = document.createElement('script'); fallback.src = fallbackUrl; fallback.onload = resolve; fallback.onerror = reject; document.head.appendChild(fallback); }; document.head.appendChild(script); }); }packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/options.ts (1)
39-46: Consider better typed defaults for block settings properties.The block settings properties use empty objects as defaults, which might be unclear to developers.
According to the
TabsOptionsinterface from the related file,tabsBlockcan beComponentDefinition | null. Consider setting it explicitly tonullor providing documentation on expected fields for these properties.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/commands.ts (2)
54-59: Implement empty methods that match their JSDoc descriptions.The
getPreContentandgetPostContentmethods are empty despite their JSDoc descriptions indicating they should return specific content types./** * Custom pre-content. Can be a simple string or an HTMLElement */ -getPreContent() {}, +getPreContent(): ContentTypes { + return undefined; +}, /** * Custom post-content. Can be a simple string or an HTMLElement */ -getPostContent() {}, +getPostContent(): ContentTypes { + return undefined; +},Also applies to: 65-78
110-120: Use specific type for codeViewer rather than 'any'.The codeViewer is typed as 'any', which prevents proper type checking.
Consider defining an interface for the code viewer that includes the methods being used (getContent, setContent, refresh, focus, getElement) to improve type safety.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/blocks.ts (1)
56-58: Add type attribute to button for proper form submission.The button in the form doesn't specify a type attribute, which defaults to "submit" in most browsers but should be explicit for clarity.
{ - components: [{ type: typeButton }], + components: [{ type: typeButton, attributes: { type: 'submit' } }], },packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts (2)
5-9: Use a more specific type for Typed.js.The global declaration for
window.Typedusesanywhich bypasses type checking.Consider defining a proper interface for the Typed.js library to improve type safety:
declare global { interface Window { Typed: { new (element: HTMLElement, options: Record<string, any>): any; } } }
69-77: Improve empty strings handling.The
getStringsfunction returns an empty array if the string doesn't contain newlines, which could be confusing.const getStrings = (value: string | string[]) => { if (Array.isArray(value)) { return value; } if (value.indexOf('\n') >= 0) { return value.split('\n'); } - return []; + return value ? [value] : []; };packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts (2)
17-75: Consider using classList for adding/removing active states.
Usingitem.className = …repeatedly might introduce bugs if class names are manipulated or if multiple classes exist. A more robust approach is to useelement.classList.add(...)orelement.classList.remove(...).- item.className = item.className.replace(classTabActive, '').trim(); + item.classList.remove(classTabActive);
94-119: Add cleanup for event listeners.
The code attaches a click event listener toelon initialization but does not detach it in a destroy/unmount phase. Consider removing the listener if the component is unmounted to avoid potential stale references or memory leaks.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts (1)
293-307: Add error handling for asset uploads.
The upload logic silently skips ifres?.data?.[0]does not contain a valid source. Consider providing a fallback or error message to the user if the server rejects the file or responds with an unexpected format.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/components.ts (1)
70-99: Avoid multiple setTimeout calls if frequent component changes occur.
The debounce logic withtimedIntervalcan be improved to better handle rapid changes (e.g., consecutive user edits) with less overhead. A more robust debouncing/throttling approach would be beneficial here.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts (3)
3-41: Use stricter typing for plugin options.
Even though TS interfaces are used, consider adding more specific types (e.g., union types fordateInputType, or required fields if you rely on them downstream) to improve type-safety and catch potential misconfigurations early.
42-99: Consider validating date strings to avoid NaN.
IfstartTimeis invalid or empty,new Date(startfrom).getTime()returnsNaN. You may want to handle and log this scenario more gracefully (e.g., show an error or default behavior) rather than letting it fall back to negative intervals.
175-209: Edge case: countdown just passed.
If the countdown date is only a few seconds in the past at load time, the countdown might show 0 days, hours, etc., for a moment before it transitions to the expired state. If that’s not desired, you could handle negative distance up front.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/components.ts (4)
16-45: Consolidate common trait definitions.
Traits likeid,name,placeholder, andvalueappear multiple times. Consider exporting them as shared objects to reduce duplicate code and maintain consistency across components.
46-54: Improve labeling for accessibility.
ThecreateOptionhelper only sets internal text as content but does not provide anyaria-attributes or labeling for screen readers. Consider adding accessibility attributes for more inclusive usage.
98-144: Autocomplete removal is helpful for text inputs.
Line 141 setsautocomplete="off". This helps in the visual editor context. However, remember that in production forms, users often expect autocomplete. Possibly make this opt-in or configurable if needed for real user flows.
301-314: Label component may need additional traits.
Currently, the only trait isfor. Consider adding a trait for nested text editing or advanced label attributes.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts (2)
54-105: Simplify merging defaults.
The current approach merges default objects with user-provided options. It’s correct but somewhat verbose. Consider using a single utility function or a partial approach with modern TS features (e.g.,Partial<RichTextEditorOptions>) to reduce boilerplate.
373-387: Potential i18n expansions for subscript/superscript icons.
If you anticipate multi-language support, consider using a neutral icon or referencing a format that doesn’t rely on'X<sub>2</sub>'text. This is minor but might be relevant for large-scale usage.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts (2)
76-76: Remove redundant assignment.Line 76 assigns
this.tabContent = content;which is redundant since this assignment already happens in line 73 when a new content is created, and the existing content is already being used when present.- this.tabContent = content;
84-85: Add null check before triggering rerender.There's no check if
getTabsType()returns a valid component before triggering the 'rerender' event. This could cause errors if the tabs parent component is removed first.__onRemove() { const content = this.getTabContent(); if (content) { content.remove(); } - this.getTabsType().trigger('rerender'); + const tabsType = this.getTabsType(); + if (tabsType) { + tabsType.trigger('rerender'); + } },packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts (1)
13-18: Consider making class name properties required.While the interface is well-structured, properties like
classTaband other class names appear to be required for the component to function properly, but they are not marked as optional (no?suffix). This correctly indicates they are required. However, to maintain consistency, consider explicitly documenting this requirement in comments or by grouping required and optional properties separately.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.changeset/three-flies-add.md(1 hunks)packages/studiocms_wysiwyg/package.json(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/components/Editor.astro(4 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/blocks.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/blocks.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/commands.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/components.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/utils.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/blocks.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/components.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/traits.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/colorPicker.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/styles.css(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/blocks.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContainer.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContent.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContents.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/options.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/blocks.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/traits.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/utils.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/traits.ts (2)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts (1)
editor(17-125)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/utils.ts (1)
traitStringId(2-2)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/traits.ts (1)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/components.ts (1)
typeOption(11-11)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/index.ts (3)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/traits.ts (1)
editor(4-19)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/blocks.ts (1)
editor(5-18)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts (1)
editor(17-125)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/blocks.ts (3)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts (1)
editor(17-125)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/index.ts (1)
RequiredPluginOptions(40-40)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/utils.ts (1)
cmpId(1-1)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContainer.ts (5)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContents.ts (1)
editor(4-20)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts (1)
editor(12-118)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/index.ts (1)
editor(20-30)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts (1)
editor(14-222)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts (1)
TabContainerConfig(52-58)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/options.ts (1)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts (3)
TabsOptions(3-31)TabTemplate(37-37)TabTemplateProps(33-35)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts (1)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/colorPicker.ts (1)
Piklor(19-230)
🔇 Additional comments (44)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts (1)
158-177: Verify block creation categories.The newly added block is placed under category
'Extra'. Confirm that it's aligned with the desired GrapesJS category or if it should be placed elsewhere (e.g.,'Basic','Components').packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/colorPicker.ts (1)
128-130: Confirm autoclose preference.
if (this.options.autoclose !== false) { this.close(); }might confuse those who expect the color picker to stay open unless explicitly closed. Consider making this behavior opt-in rather than opt-out..changeset/three-flies-add.md (1)
1-5: Well-structured changeset with clear reason for dependency changes.The changeset properly documents the patch update for
@studiocms/wysiwygand clearly explains the rationale behind merging GrapesJS open-source plugins into the codebase. This is a good practice for tracking changes and communicating the purpose of modifications.packages/studiocms_wysiwyg/package.json (1)
70-71:Details
✅ Verification successful
Simplified dependency management approach.
The changes to the dependencies section align with the changeset's stated purpose of merging GrapesJS plugins directly into the codebase. Adding
tui-image-editoras a direct dependency while removing multiple external GrapesJS plugin dependencies (as indicated in the AI summary) should help resolve the build errors mentioned in the changeset.Consider documenting the specific version constraints for all direct dependencies to ensure consistent builds across different environments.
🏁 Script executed:
#!/bin/bash # Verify tui-image-editor usage in the codebase rg -l "tui-image-editor" --type ts --type js packages/studiocms_wysiwyg/src/Length of output: 147
Dependency Addition Verified & Documentation Suggestion
- The updated dependency section now adds
tui-image-editoras a direct dependency, which aligns with merging GrapesJS plugins into the codebase. This is confirmed by its usage inpackages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts.- This simplified dependency management should alleviate the previously encountered build errors.
- Consider documenting explicit version constraints for all direct dependencies to ensure consistent builds across different environments.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/utils.ts (1)
1-2: Good practice using constants for component identifiers.Using constant variables for component and trait identifiers promotes code maintainability and reduces the risk of typos when referencing these values throughout the codebase.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/index.ts (3)
1-6: Well-structured imports with clear organization.The imports are well-organized, separating types, component loaders, and configuration options. Using explicit file extensions in imports is a good practice for module resolution clarity.
9-20: Clean plugin initialization with proper options handling.The plugin implementation follows good practices:
- Proper typing with
Plugin<Partial<PluginOptions>>- Merging default options with user-provided options
- Clear separation of concerns by delegating component and block loading to separate modules
- Concise implementation with clear comments
This structure makes the code maintainable and follows the standard GrapesJS plugin pattern.
7-8: Clean exports with proper type exposure.The exports are handled appropriately - exporting the plugin as default and re-exporting the
PluginOptionstype for external consumption. This provides a clean interface for users of this plugin.Also applies to: 22-22
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/traits.ts (1)
1-19: Clean trait implementation for handling string arraysThis is a well-structured implementation of a custom trait for the GrapesJS editor. The trait provides a way to edit an array of strings through a textarea interface, with appropriate conversion between newline-separated text and the array format.
Good practices observed:
- Proper handling of empty values with
(elInput.value || '').split('\n')- Clean separation of input creation, update, and event handling
- Following GrapesJS trait API patterns
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/blocks.ts (1)
5-17: Block registration looks goodThe block registration properly implements conditional creation based on the options provided and includes all necessary properties like label, icon, and content type. The spread operator on line 16 allows for flexible customization of the block.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/blocks.ts (1)
5-21: Well-implemented custom code blockThe implementation correctly registers a Custom Code block with appropriate defaults and customization options. Setting
activate: trueandselect: trueprovides a good user experience by automatically focusing the block when added.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/index.ts (2)
11-18: Well-organized component loading sequence.The explicit component loading order is a smart approach to ensure proper dependency resolution. This prevents potential initialization errors that could occur if components were loaded in an arbitrary order.
20-30: Clean implementation with proper type safety.The implementation effectively merges configuration with editor defaults and correctly passes them to each component loader. The type safety ensures components receive the expected parameters.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/index.ts (1)
29-40: LGTM: Good plugin structure with proper configuration handling.The plugin correctly merges default and user-provided options, and properly initializes form components, traits, and blocks in sequence.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/index.ts (2)
4-94: Excellent option documentation and plugin configurability.The comprehensive JSDoc documentation for each option makes this plugin highly maintainable. The default values are clearly documented, and the configuration options provide good flexibility for customization.
96-117: Well-structured plugin implementation.The implementation correctly merges user options with sensible defaults and passes the configuration to the block loader. This approach ensures consistency while allowing for customization.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/index.ts (1)
6-38: Well-documented interface with helpful usage example.The plugin options are clearly documented with a particularly useful example for the
propscustomization function. This makes the plugin highly configurable and easier for developers to use effectively.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/TabContainer.ts (2)
4-4: Good use of ARIA role for accessibility.Using 'tablist' as the ARIA role is the correct implementation for tab containers, which improves accessibility for users with screen readers.
6-24: Well-structured component implementation.The TabContainer implementation follows good practices:
- Clear separation of concerns
- Proper type definitions with TypeScript
- Appropriate constraints for dragging and dropping
- Correctly disables copy/remove operations to maintain tab integrity
- Extends with config properties for flexibility
This component pairs well with the other tab components to create a cohesive tabs system.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/blocks.ts (2)
57-83: Good use of conditional configuration based on grid type.The code correctly adjusts resizer behavior based on the grid implementation (flexbox vs table), which ensures proper user experience when resizing elements. Setting
keyWidthto'flex-basis'for flex elements is the appropriate approach.
104-109: Effective use of event listening for selector privacy.The implementation makes row and column CSS selectors private to prevent user modification, which protects the structural integrity of the grid system.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/index.ts (2)
6-56: Well-documented plugin options with comprehensive JSDoc.The
PluginOptionstype is thoroughly documented with JSDoc comments, providing clear information about each option including examples. This makes the API more user-friendly and self-documenting.
58-84: Clean and modular plugin architecture.This implementation follows a modular approach by:
- Separating concerns into components, blocks, and commands
- Providing sensible defaults that can be overridden
- Using a well-typed API with proper TypeScript types
- Organizing code into logical modules
This approach makes the code more maintainable and easier to extend.
packages/studiocms_wysiwyg/src/wysiwyg/components/Editor.astro (3)
4-4: Updated CSS import to use local implementation.The CSS import has been changed from an external package to a local implementation, which aligns with the overall goal of internalizing dependencies.
45-54: Successfully migrated to local plugin implementations.All GrapesJS plugin imports have been updated to use local implementations instead of external packages, which:
- Reduces external dependencies
- Gives more control over the codebase
- Allows for customization without forking external packages
This is a solid strategy for long-term maintainability.
195-221:Details
❓ Verification inconclusive
Reordered plugin initialization with potential side effects.
The order of plugin initialization has changed, and some plugins have been removed entirely. While this may be intentional, it could introduce side effects if plugins have interdependencies.
Please verify that all functionality still works correctly, particularly features that might have depended on removed plugins like
grapesjs-plugin-export,grapesjs-parser-postcss,grapesjs-style-bg, andgrapesjs-touch.
🏁 Script executed:
#!/bin/bash # Check for any remaining references to removed plugins echo "Checking for references to removed plugins..." rg -i "grapesjs-plugin-export|grapesjs-parser-postcss|grapesjs-style-bg|grapesjs-touch" --type ts --type jsLength of output: 210
Plugin Initialization Order Updated – Verify Runtime Behavior
The updated plugin initialization order now omits several plugins, which might affect interdependencies. Our initial search in TypeScript/JavaScript files didn’t reveal any stray references to
grapesjs-plugin-export,grapesjs-parser-postcss,grapesjs-style-bg, orgrapesjs-touch. However, to be thorough, please:
- Manually confirm that these references aren’t present in other file types (e.g.,
.astrofiles).- Verify that all functionality still operates correctly, especially features that might have relied on the removed plugins.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/options.ts (2)
71-83: Function implementation is solid.The
resolveTemplatefunction correctly handles different input types and undefined values.
85-87: Function implementation is correct.The
mergeStylesfunction properly handles the case where a user style is not provided.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts (1)
161-176: Ensure consistent tab indexing upon dynamic additions.
When tabs are added, the logic incrementstabCountbut relies on the same approach for indexing. Verify that external references to specific tabs remain valid if a user inserts or deletes tabs in varying sequences.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/components.ts (1)
10-19: Confirm intended removal of<script>content.
By clearing the innerHTML of'script'components nested in custom code, you effectively disable script rendering. Ensure this aligns with your security or editorial requirements, as some users might expect script code to remain for demonstration.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts (2)
141-161: Potential repeated interval clearing.
The code correctly clears old intervals (oldInterval && clearInterval(oldInterval)), but ensure that any newly created interval is also reliably cleared if the countdown element is removed or re-rendered. The logic in lines 162–173 helps maintain a global cleanup, which is good.
211-285: Block and component registration look good.
The introduced block, component defaults, and traits are well-structured and consistent with GrapesJS conventions. The inlined styles are minimal and clearly scoped, which avoids global CSS collisions.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/components.ts (3)
56-96: Preventing form submission is correct but might mask some editor flows.
The defaultsubmithandler is prevented to avoid navigating away during editing, which is typical in GrapesJS. Ensure you provide an alternate means of testing final form submission if that’s relevant for your use case.
203-230: Checkbox handling is well done.
The approach for binding thecheckedattribute via trait changes is a standard GrapesJS pattern. Good job on hooking event listeners that respect preview mode.
243-299: Dynamic button text updates.
Usingthis.components(this.get('text'))in the button model syncs the internal text structure effectively. This is a neat pattern to keep buttons WYSIWYG-friendly.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts (5)
1-52: Leverage default TypeScript types for plugin configurations.
You already define several interfaces for the RTE options. Confirm that you’re leveraging them in the plugin’s callback signature (Plugin<RichTextEditorOptions>) for thorough compile-time checks.
141-164: Font name dropdown is straightforward.
The logic for adding a new command with theselectelement to handle font name changes is well-implemented. This is a good demonstration of customizing the GrapesJS RTE.
273-356: Heading & formatting commands are well laid out.
TheformatBlockusage with<h1>through<h6>or<p>is standard. Also, removing unneeded commands based on user config is a nice approach for a flexible toolbar.
491-506: Undo/redo features are essential.
Thanks for making them optional. This is particularly handy for large editors or minimal WYSIWYG configurations. The condition-based approach is clean and consistent with the other features.
197-234:Details
✅ Verification successful
Check color picker initialization.
You’re creating newPiklorinstances on demand. Ensure repeated calls to the same UI element do not cause memory leaks or overlapping event listeners.To verify, search for references to multiple calls of
'fontColor'and'hiliteColor'in the codebase to confirm they are never re-initialized for the same element:
🏁 Script executed:
#!/bin/bash rg -A 5 "rte.add\('fontColor'" rg -A 5 "rte.add\('hiliteColor'"Length of output: 1303
Memory Picker Initialization Verified
The search results confirm that the
rte.add('fontColor')andrte.add('hiliteColor')definitions appear only once in the codebase. Moreover, the guard condition (if (!pk1[pikerEle])) in theresultcallback prevents creating multiplePiklorinstances for the same element, which mitigates potential memory leaks or overlapping event listeners.No changes are required at this time.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts (2)
54-58: Well-implemented ARIA attributes for accessibility.The component correctly implements ARIA attributes for tab functionality, making the tabs accessible. The use of
aria-labelledbyand proper tab/tabpanel relationships follows best practices.
102-115: Well-structured clone method.The clone method is well implemented to handle the complex case of cloning tabs while maintaining the relationships between tabs and their content panels. The handling of different cloning contexts (from parent or not) is particularly thoughtful.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts (2)
3-31: Well-structured configuration interfaces.The
TabsOptionsinterface is comprehensively designed with clear organization of different aspects of tab configuration (block settings, class names, selectors/types, and templates). The separation into categories with comments improves readability.
89-95: Good accessibility typing with TabAttributes.The
TabAttributesinterface properly defines ARIA attributes needed for accessible tab implementations, following web accessibility best practices. The typing will help ensure proper use of these attributes throughout the codebase.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts
Show resolved
Hide resolved
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts
Show resolved
Hide resolved
…Picker.ts Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts (1)
176-203: Reiterate the importance of SRI or additional error handling for dynamic script loading.This matches a previous recommendation to add Subresource Integrity checks or more robust success/error handling to prevent silent failures and security issues.
🧹 Nitpick comments (1)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts (1)
274-276: Provide a type-safe approach for theme overrides.Accessing
config.includeUI.theme['header.display']via string indexing is a workaround. Consider extending the TOAST UI theme definitions to avoid@ts-ignoreand support stronger typing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/LICENSE(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/LICENSE(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/LICENSE(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/LICENSE(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/LICENSE(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/LICENSE(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/LICENSE
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/LICENSE
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/LICENSE
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/LICENSE
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/LICENSE
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/LICENSE
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (13)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts (5)
17-23: This script function should have JSDoc commentsThe script function is complex and serves as the UI controller for the tabs component. Adding JSDoc comments would make its purpose clearer and document the expected props structure.
+/** + * Client-side controller script for the tabs component. + * @param {TabComponentProps} props - Properties passed to the script + * @param {string} props.classactive - CSS class for active tab + * @param {string} props.selectortab - Attribute used to select tab + */ const script = function (this: HTMLElement, props: TabComponentProps): void {
32-36: Consider creating type definitions instead of usinganyThe code uses
anytype with biome-ignore comments for browser compatibility. Creating a proper interface for these vendor-specific methods would improve type safety.+interface HTMLElementWithVendorMatches extends HTMLElement { + webkitMatchesSelector?: typeof HTMLElement.prototype.matches; + mozMatchesSelector?: typeof HTMLElement.prototype.matches; + msMatchesSelector?: typeof HTMLElement.prototype.matches; +} const matches = (body as HTMLElement).matches || - // biome-ignore lint/suspicious/noExplicitAny: <explanation> - (body as any).webkitMatchesSelector || - // biome-ignore lint/suspicious/noExplicitAny: <explanation> - (body as any).mozMatchesSelector || - // biome-ignore lint/suspicious/noExplicitAny: <explanation> - (body as any).msMatchesSelector; + (body as HTMLElementWithVendorMatches).webkitMatchesSelector || + (body as HTMLElementWithVendorMatches).mozMatchesSelector || + (body as HTMLElementWithVendorMatches).msMatchesSelector;
97-122: Consider simplifying the click event handling logicThe click event handler is complex with nested conditions and a specific event prevention mechanism using
__trg. This code could be made more readable by extracting some logic into separate functions.el.addEventListener('click', (ev: MouseEvent & { __trg?: number }) => { let target = ev.target as HTMLElement | null; if (!target) return; + const isTabElement = (element: HTMLElement): boolean => matches.call(element, roleTab); + + const findClickableTab = (element: HTMLElement): HTMLElement | null => { + if (isTabElement(element)) return element; + const selectedTab = getSelectedTab(element); + return selectedTab || null; + }; + + const tabElement = findClickableTab(target); + + if (tabElement && !ev.__trg && tabElement.className.indexOf(classTabActive) < 0) { + ev.preventDefault(); + ev.__trg = 1; + activeTab(tabElement); + const id = getTabId(tabElement); + try { + history?.pushState(null, '', `#${id}`); + } catch (e) { + console.error('History API is not supported', e); + } + } - let found = matches.call(target, roleTab); - - if (!found) { - const selectedTab = getSelectedTab(target); - if (selectedTab) { - target = selectedTab; - found = true; - } - } - - if (found && !ev.__trg && (target as HTMLElement).className.indexOf(classTabActive) < 0) { - ev.preventDefault(); - ev.__trg = 1; - activeTab(target as HTMLElement); - const id = getTabId(target as HTMLElement); - try { - history?.pushState(null, '', `#${id}`); - } catch (e) { - console.error('History API is not supported', e); - } - } });
161-163: Improve type predicate implementationThe type predicate function uses
anyfor the input parameter. It would be better to useunknownfor better type safety.- // biome-ignore lint/suspicious/noExplicitAny: <explanation> - isTabsComponent(comp: any): comp is TabsComponent { + isTabsComponent(comp: unknown): comp is TabsComponent { return comp !== null && typeof comp === 'object' && 'findTabs' in comp; },
182-187: Fix type annotations and provide better documentationThe
__onTabmethod usesanyfor parameter types, and the method lacks documentation explaining its purpose. This method is crucial for tab initialization.- // biome-ignore lint/suspicious/noExplicitAny: <explanation> - __onTab(tab: TabComponent, index: number, v: any, opts: any = {}) { + /** + * Initializes a tab component with the given index + * @param tab - The tab component to initialize + * @param index - The index of the tab + * @param v - Unused parameter (consider removing) + * @param opts - Options including avoidStore and temporary flags + */ + __onTab( + tab: TabComponent, + index: number, + v: unknown, + opts: { avoidStore?: boolean; temporary?: boolean } = {} + ) { if (!opts.avoidStore && !opts.temporary) { tab.__initTab(index); } },packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts (4)
15-17: Consider using constants for attribute namesThe attribute name
data-previs used as a string literal. Consider defining it as a constant at the top level of the module to ensure consistency and easier updates.-const prevSelectorTab = 'data-prev'; +const PREV_SELECTOR_TAB = 'data-prev'; -const getPrevCntId = (comp: Component) => comp.getAttributes()[prevSelectorTab]; +const getPrevCntId = (comp: Component) => comp.getAttributes()[PREV_SELECTOR_TAB];
47-77: Reduce code duplication in __initTab methodThere's duplicate code for setting attributes in the two conditional branches. Consider extracting this logic to a helper function.
__initTab(index: number) { if (this.tabContent) return; let content = this.getTabContent(); + const setupTabContentRelationship = (tabContent: Component) => { + const id = tabContent.getId(); + const tabId = this.getId(); + tabContent.addAttributes({ id, 'aria-labelledby': tabId, hidden: true }); + this.addAttributes({ [config.selectorTab]: id, id: tabId }); + return tabContent; + }; if (content && getPrevCntId(content)) { const id = content.getId(); const tabId = this.getId(); content.addAttributes({ id, 'aria-labelledby': tabId, hidden: true }); content.removeAttributes(prevSelectorTab); this.addAttributes({ [config.selectorTab]: id, id: tabId }); } if (!content) { const tabs = this.getTabsType(); const cnts = tabs.getContentsType(); content = cnts.append({ type: config.typeTabContent, components: resolveTemplate(config.templateTabContent, { index: index, }), })[0]; - const id = content.getId(); - const tabId = this.getId(); - content.addAttributes({ id, 'aria-labelledby': tabId, hidden: true }); - this.addAttributes({ [config.selectorTab]: id, id: tabId }); + content = setupTabContentRelationship(content); this.tabContent = content; } this.tabContent = content; },
79-85: Add error handling to __onRemove methodThe __onRemove method removes the associated tab content but doesn't have error handling for potential issues like the tabs type not being available.
__onRemove() { const content = this.getTabContent(); if (content) { content.remove(); } - this.getTabsType().trigger('rerender'); + const tabsType = this.getTabsType(); + if (tabsType) { + tabsType.trigger('rerender'); + } },
102-116: Improve type safety in clone methodThe clone method uses
anyfor the rest parameters and has a complex cloning logic that could benefit from better type annotations and documentation.- // biome-ignore lint/suspicious/noExplicitAny: <explanation> - clone(opt: { _inner?: boolean; symbol?: boolean; symbolInv?: boolean } = {}, ...args: any[]) { + /** + * Clone the tab component, handling the relationship with tab content + * @param opt - Cloning options + * @param args - Additional arguments passed to the base clone method + * @returns The cloned component + */ + clone( + opt: { _inner?: boolean; symbol?: boolean; symbolInv?: boolean } = {}, + ...args: unknown[] + ): Component { const fromParent = opt._inner; if (fromParent) { const tabCont = this.getTabContent(); if (tabCont) { - tabCont.addAttributes({ [prevSelectorTab]: tabCont.getId() }); + tabCont.addAttributes({ [prevSelectorTab]: tabCont.getId() || '' }); } } const cloned = defaultModel.prototype.clone.apply(this, [opt, ...args]); if (!fromParent) { cloned.addAttributes({ [config.selectorTab]: '' }); } return cloned; },packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts (4)
1-3: Imports look good, but consider using consistent extension patterns.The import for ColorPicker uses .js extension which is fine for ES modules, but make sure this is consistent with the rest of the codebase's import patterns.
273-355: Significant code duplication in formatting options.There's considerable repetition in the code for adding heading buttons. Consider using a helper function to reduce duplication.
+ // Helper function to add heading buttons + const addHeadingButton = (level: number, enabled: boolean) => { + if (!options.format || !enabled) return; + + rte.add(`heading${level}`, { + icon: icons?.[`heading${level}`] || `<div>H${level}</div>`, + attributes: { + title: `Heading ${level}`, + }, + result: (rte) => rte.exec(formatBlock, `<h${level}>`), + }); + }; + + // Add heading buttons + for (let i = 1; i <= 6; i++) { + addHeadingButton(i, formatOpts[`heading${i}`]); + }
388-403: List implementation could benefit from a similar refactoring approach.Like the heading buttons, the list implementation has duplicated structure that could be reduced with a helper function.
1-511: Consider adding JSDoc comments for better developer documentation.The code would benefit from JSDoc comments, especially for the main plugin function and the interfaces. This would make it easier for other developers to understand how to use and extend this plugin.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.vscode/settings.json(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/blocks.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/commands.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/components.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/components.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/traits.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/colorPicker.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts(1 hunks)packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- .vscode/settings.json
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tuiImageEditor.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/countdown.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/traits.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/blocks/blocks.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/commands.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/typed/components.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tooltip.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/components.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/forms/components.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/code/index.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/types.ts
- packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/colorPicker.ts
🔇 Additional comments (9)
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tabs.ts (2)
57-61: Good practice handling tabindex differently in editor modeThe implementation correctly disables tabindex changes when in editor mode to maintain editor functionality. This is a valuable approach for preventing UI conflicts between the editor and the component behavior.
205-224: Good implementation of the addTab methodThe
addTabmethod is well-implemented with flexible content handling. It supports string content, template function, or default templates. This promotes reusability and customization.packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/tabs/components/Tab.ts (2)
24-26: Good use of ARIA roles for accessibilityThe component properly sets the ARIA role to "tab" for accessibility, which is essential for screen readers and other assistive technologies to identify the tab component correctly.
29-39: Well-implemented styling trait buttonThe "Style Active" trait button is a good UX enhancement allowing users to easily style the active tab state through the Style Manager. This promotes a cleaner editing experience.
packages/studiocms_wysiwyg/src/wysiwyg/editorPlugins/rte/index.ts (5)
4-52: Well-structured TypeScript interfaces for configuration options.The interfaces are well-defined and provide good type safety for configuring the Rich Text Editor. The hierarchy of options (font settings, basic formatting, paragraph formatting, etc.) makes the plugin highly configurable.
54-106: Default configuration provides sensible starting values.The default options are well-considered, enabling common text formatting features while disabling more advanced ones by default. The commented options provide helpful documentation for users who want to customize the plugin.
133-141: Good conditional configuration based on user options.The code correctly handles disabling default features when they're not needed, which helps keep the toolbar clean and focused.
356-387: Indent/outdent and subscript/superscript implementation looks good.The implementation for these formatting options is straightforward and correctly implements the necessary functionality.
475-506: Code and line insertion plus undo/redo features are implemented correctly.The implementation for code blocks, horizontal lines, and undo/redo functionality follows the same pattern as the other buttons and appears correct.
fixes: https://discordapp.com/channels/1309279407743172608/1361400777955868752
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Style
Chores