fix external packages | fix text editor #72
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe updates primarily involve refactoring and exporting components in a Vue.js library, with a focus on form elements like calendars, date pickers, and text editors. Changes include adjusting import paths, removing TypeScript annotations, and simplifying code by removing unnecessary commas. A new Changes
Poem
TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (10)
- packages/@orchidui-vue/src/ComplexCalendar.js (1 hunks)
- packages/@orchidui-vue/src/ComplexDatePicker.js (1 hunks)
- packages/@orchidui-vue/src/Elements/Sidebar/OcSidebar.stories.js (1 hunks)
- packages/@orchidui-vue/src/Form/ComplexCalendar/OcComplexCalendar.stories.js (1 hunks)
- packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (1 hunks)
- packages/@orchidui-vue/src/Form/index.js (1 hunks)
- packages/@orchidui-vue/src/TextEditor.js (1 hunks)
- packages/@orchidui-vue/vite.config.js (2 hunks)
Files skipped from review due to trivial changes (5)
- packages/@orchidui-vue/src/ComplexCalendar.js
- packages/@orchidui-vue/src/ComplexDatePicker.js
- packages/@orchidui-vue/src/Elements/Sidebar/OcSidebar.stories.js
- packages/@orchidui-vue/src/Form/ComplexCalendar/OcComplexCalendar.stories.js
- packages/@orchidui-vue/src/Form/index.js
Additional comments: 6
packages/@orchidui-vue/src/TextEditor.js (1)
- 1-1: The export statement correctly re-exports all named exports from the specified module. Ensure that the file path is correct and that the module at
./form/TextEditor/OcTextEditor.jsis intended to be fully exposed as part of the public API.packages/@orchidui-vue/vite.config.js (2)
10-17: The addition of the
externalComponentsarray is a good approach to manage external dependencies. Ensure that the components listed are the only ones that should be excluded from the build process and that they are correctly referenced elsewhere in the project.30-35: The updated filter logic now excludes files with
.sample,.stories, and those listed inexternalComponentsfrom the build entries. Verify that this change does not inadvertently exclude any necessary files that should be included in the build.packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (1)
- 1-6: The import statements have been updated to reflect the new file structure. Ensure that the new paths are correct and that the components are correctly exported from those paths. Also, verify that the
ComplexCalendarcomponent is indeed located at'@/orchidui/ComplexCalendar'and that this change is consistent with the rest of the codebase.Overall, the changes seem to be aimed at improving the modularity and maintainability of the code by ensuring that components are imported from their specific paths rather than a general index file. This can help with tree shaking and potentially reduce the bundle size if the build system supports it.
packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js (1)
- 1-2: The import statements have been updated to use named imports and reflect the new file paths. Verify that the new paths are correct and that the
TextEditorcomponent is indeed exported as a named export from "@/orchidui/TextEditor".packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (1)
- 1-4: The removal of TypeScript from the script setup and the addition of Vue Quill component imports are significant changes. Ensure that the removal of TypeScript does not affect the type safety and that the project's build process is updated accordingly if TypeScript was previously being transpiled. The addition of Vue Quill should be accompanied by an update to the project's documentation and possibly its dependencies in the
package.jsonfile.The use of
onMountedandreffrom Vue indicates that lifecycle hooks and reactive references are being utilized. Ensure that theonMountedhook is used correctly and that all references are properly managed within the component's lifecycle.The import of Vue Quill's CSS file at the top of the script setup ensures that the styles are loaded with the component. This is a good practice for component encapsulation.
The rest of the script setup code appears to be business logic related to the text editor's functionality, such as formatting options and event handlers. Ensure that this logic is thoroughly tested, especially since TypeScript has been removed, which could potentially lead to type-related bugs if not handled carefully.
packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (3)
- packages/@orchidui-vue/src/Form/SingleFileUpload/ModalCropper.vue (1 hunks)
- packages/@orchidui-vue/src/Form/index.js (1 hunks)
- packages/@orchidui-vue/vite.config.js (2 hunks)
Files skipped from review due to trivial changes (2)
- packages/@orchidui-vue/src/Form/SingleFileUpload/ModalCropper.vue
- packages/@orchidui-vue/vite.config.js
Additional comments: 1
packages/@orchidui-vue/src/Form/index.js (1)
- 13-21: The removal of exports for
OcTextEditor.js,OcComplexCalendar.js,OcComplexDatePicker.js, andOcSingleFileUpload.jsshould be verified to ensure that these components are not used elsewhere in the project or by external consumers who might be relying on these exports. If they are, this could be a breaking change.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (5)
- packages/@orchidui-vue/src/ComplexCalendar.js (1 hunks)
- packages/@orchidui-vue/src/ComplexDatePicker.js (1 hunks)
- packages/@orchidui-vue/src/SingleFileUpload.js (1 hunks)
- packages/@orchidui-vue/src/TextEditor.js (1 hunks)
- packages/@orchidui-vue/vite.config.js (1 hunks)
Files skipped from review due to trivial changes (3)
- packages/@orchidui-vue/src/ComplexCalendar.js
- packages/@orchidui-vue/src/SingleFileUpload.js
- packages/@orchidui-vue/src/TextEditor.js
Additional comments: 2
packages/@orchidui-vue/src/ComplexDatePicker.js (1)
- 1-3: The change correctly updates the import path and exports the
ComplexDatePicker. Ensure that all references toComplexDatePickerthroughout the project are updated to use this new export path.packages/@orchidui-vue/vite.config.js (1)
- 24-29: The removal of the check for ".stories" files and the specific "HitpaySidebar" naming convention simplifies the file filtering logic. Ensure that this change does not inadvertently include files that should be excluded from the build process.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (2)
- package-lock.json
- package.json
Files selected for processing (7)
- packages/@orchidui-vue/src/Form/SingleFileUpload/OcSingleFileUpload.stories.js (1 hunks)
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js (2 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (5 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/options.js (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/snow.css (1 hunks)
Files skipped from review due to trivial changes (4)
- packages/@orchidui-vue/src/Form/SingleFileUpload/OcSingleFileUpload.stories.js
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue
- packages/@orchidui-vue/src/Form/TextEditor/options.js
- packages/@orchidui-vue/src/Form/TextEditor/snow.css
Additional comments: 7
packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js (2)
2-2: The import statement for
TextEditorhas been updated to reflect the local import path. This change is consistent with the pull request summary stating that import paths have been updated to reflect new file paths or component structures.31-37: The
setupfunction has been updated to initializemodelValuewith a default string value. This is a good practice as it sets an initial state for the model value which can be useful for demonstrating the component in a story or for initializing the component with a default value in a real use case.packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (5)
1-8: The removal of TypeScript (
lang="ts") and the switch to a regularsetupscript should be verified to ensure that all TypeScript-specific code has been refactored or removed, and that the component still functions as expected without type checking.23-29: The registration of the
Sizeattribute and the setup of reactive references look correct. However, ensure that thefontSizesprop is provided with the correct format as expected by theSize.whitelistmapping.50-60: The
undoandredofunctions have been updated to uselocalValue.valueinstead ofprops.modelValue. This change assumes thatlocalValuewill always be in sync with the editor's content. Ensure that there are mechanisms in place (like watchers or computed properties) to keeplocalValueupdated with the editor's state.150-156: The
QuillEditorcomponent'smodel-valueprop has been replaced withcontent. This change should be verified to ensure that theQuillEditorcomponent is correctly using thecontentprop to control its internal state and that it emits updates as expected.349-355: The import of the
snow.cssfile and the styling for.ql-containerare correct. Ensure that thesnow.cssfile is located at the correct relative path from this component.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (28)
- packages/@orchidui-vue/src/DataDisplay/Pagination/OcPagination.vue (4 hunks)
- packages/@orchidui-vue/src/Disclosure/Accordion/OcAccordion.stories.js (1 hunks)
- packages/@orchidui-vue/src/Elements/Header/OcTabToSelect.vue (1 hunks)
- packages/@orchidui-vue/src/Elements/PageTitle/OcPrimaryActions.vue (1 hunks)
- packages/@orchidui-vue/src/Form/Button/OcButton.vue (3 hunks)
- packages/@orchidui-vue/src/Form/Calendar/OcCalendar.vue (9 hunks)
- packages/@orchidui-vue/src/Form/CheckboxesGroup/OcCheckboxesGroup.stories.js (1 hunks)
- packages/@orchidui-vue/src/Form/CheckboxesGroup/OcCheckboxesGroup.vue (1 hunks)
- packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (2 hunks)
- packages/@orchidui-vue/src/Form/LinkInput/OcLinkInput.vue (1 hunks)
- packages/@orchidui-vue/src/Form/PhoneInput/OcPhoneInput.vue (1 hunks)
- packages/@orchidui-vue/src/Form/Select/OcSelect.vue (3 hunks)
- packages/@orchidui-vue/src/Form/SingleFileUpload/ModalCropper.vue (2 hunks)
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue (2 hunks)
- packages/@orchidui-vue/src/Form/Slider/OcSlider.vue (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js (2 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (5 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/options.js (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/snow.css (1 hunks)
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePicker.vue (1 hunks)
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePopup.vue (6 hunks)
- packages/@orchidui-vue/src/Form/index.js (1 hunks)
- packages/@orchidui-vue/src/Overlay/Popper/OcPopper.vue (2 hunks)
- packages/@orchidui-vue/src/Overlay/Tooltip/OcTooltip.vue (2 hunks)
- packages/@orchidui-vue/src/Theme/OcTheme.vue (1 hunks)
- packages/@orchidui-vue/src/composables/uploadFileProgress.js (2 hunks)
- packages/@orchidui-vue/vite.config.js (1 hunks)
Files skipped from review due to trivial changes (21)
- packages/@orchidui-vue/src/DataDisplay/Pagination/OcPagination.vue
- packages/@orchidui-vue/src/Disclosure/Accordion/OcAccordion.stories.js
- packages/@orchidui-vue/src/Elements/Header/OcTabToSelect.vue
- packages/@orchidui-vue/src/Elements/PageTitle/OcPrimaryActions.vue
- packages/@orchidui-vue/src/Form/Button/OcButton.vue
- packages/@orchidui-vue/src/Form/Calendar/OcCalendar.vue
- packages/@orchidui-vue/src/Form/CheckboxesGroup/OcCheckboxesGroup.stories.js
- packages/@orchidui-vue/src/Form/LinkInput/OcLinkInput.vue
- packages/@orchidui-vue/src/Form/PhoneInput/OcPhoneInput.vue
- packages/@orchidui-vue/src/Form/Select/OcSelect.vue
- packages/@orchidui-vue/src/Form/SingleFileUpload/ModalCropper.vue
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue
- packages/@orchidui-vue/src/Form/Slider/OcSlider.vue
- packages/@orchidui-vue/src/Form/TextEditor/options.js
- packages/@orchidui-vue/src/Form/TextEditor/snow.css
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePicker.vue
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePopup.vue
- packages/@orchidui-vue/src/Overlay/Popper/OcPopper.vue
- packages/@orchidui-vue/src/Overlay/Tooltip/OcTooltip.vue
- packages/@orchidui-vue/src/Theme/OcTheme.vue
- packages/@orchidui-vue/src/composables/uploadFileProgress.js
Additional comments: 11
packages/@orchidui-vue/vite.config.js (1)
- 22-26: The change in the filter logic seems to be intentional, as per the summary provided. However, ensure that the removal of the condition to exclude "HitpaySidebar" is intentional and does not affect the build process or the inclusion of unwanted components in the final build.
packages/@orchidui-vue/src/Form/CheckboxesGroup/OcCheckboxesGroup.vue (1)
- 30-34: The removal of the comma after the spread operator is correct as it is not required and could have been a syntax error if the linter settings were strict. This change should not affect the functionality of the code, but it's important to ensure that the
modelValueis always an array when using the spread operator to avoid runtime errors.packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (2)
3-3: The import of
ComplexCalendarhas been updated to reflect its new location. Verify that the new path is correct and that the component is correctly exported from that location.44-46: The template string interpolation has been modified. Ensure that the new interpolation logic correctly formats the date range as intended and that there are no edge cases or locales where this formatting might break.
packages/@orchidui-vue/src/Form/index.js (1)
- 13-21: The removal of exports for
OcTextEditor,OcComplexCalendar,OcComplexDatePicker, andOcSingleFileUploadmodules should be verified to ensure that this does not break any existing imports in the codebase where these components are used. If these components are being used elsewhere, the import statements in those files will need to be updated to reflect the new import paths.packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.stories.js (1)
- 32-32: The
componentsobject within therenderfunction is referencingTextEditor, but the import statement on line 2 uses the default import syntax forOcTextEditor.vue. Ensure that the component is correctly registered within thecomponentsobject.packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (5)
1-8: The removal of TypeScript (
lang="ts") from the script setup block indicates a shift away from TypeScript or a simplification of the codebase. Ensure that this change is intentional and that all TypeScript-specific code has been refactored or removed.23-29: The
idis generated usingcrypto.randomUUID()and sanitized to remove non-alphabetic characters. This is a good practice for generating unique identifiers that are also DOM-friendly.50-60: The
checkStatesfunction is updated to uselocalValue.valueinstead ofprops.modelValue. This change should be verified across the application to ensure that the new local state management does not introduce any regressions or unexpected behavior.150-156: The
QuillEditorcomponent'smodel-valueprop has been renamed tocontent. This change should be reflected in all places whereQuillEditoris used to ensure consistency and prevent any issues due to the prop name change.352-355: The import of the
snow.cssfile using@importin the<style>block is a change that should be verified to ensure that the CSS file is available at the given path and that it is being correctly applied to the component.
kewcoder
changed the title
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (3)
- package-lock.json
- package.json
- packages/@orchidui-vue/package.json
Files selected for processing (1)
- packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1 hunks)
Additional comments: 1
packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1)
- 1-388: This is a comprehensive addition of a new Vue component for integrating the Quill rich text editor. The component is well-structured with clear prop definitions, event emissions, and methods for interacting with the Quill editor. Here are some points to consider:
- Ensure that the
Quilllibrary is compatible with the Vue version in use and that it's properly included in the project dependencies.- The
registerModulefunction (lines 92-99) checks if a module is already registered with Quill before registering it. This is good practice to avoid unnecessary re-registrations.- The
initializefunction (lines 102-134) sets up the Quill editor, registers any custom modules, and attaches event handlers. It's important to ensure that all event handlers are properly cleaned up in theonBeforeUnmountlifecycle hook to prevent memory leaks.- The
composeOptionsfunction (lines 137-183) merges various sources of options, which allows for flexible configuration of the Quill editor. It's important to ensure that the merging of options doesn't lead to unexpected overwrites.- The
handleTextChange,handleSelectionChange, andhandleEditorChangemethods (lines 212-246) emit events that can be listened to by the parent component. This is a good use of Vue's custom event system.- The
getContentsandsetContentsmethods (lines 267-292) handle different content types (html,text,delta). It's important to ensure that the content is properly sanitized to prevent XSS attacks when dealing with HTML content.- The
focusandreinitmethods (lines 327-338) provide ways to programmatically focus the editor and reinitialize it, which can be useful in certain scenarios.- The
watchfunctions (lines 340-362) are used to react to changes in props, which is a standard Vue pattern. However, it's important to ensure that these watchers do not cause unnecessary re-renders or performance issues.- The
renderfunction (lines 379-386) uses Vue'shfunction to create virtual DOM nodes, which is the recommended approach in Vue 3 for render functions.Overall, the component seems to be well thought out and follows Vue's composition API patterns. However, it's crucial to test the component thoroughly, especially the dynamic parts like prop changes and event handling, to ensure that it behaves as expected in different scenarios.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files ignored due to filter (3)
- package-lock.json
- package.json
- packages/@orchidui-vue/package.json
Files selected for processing (2)
- packages/@orchidui-vue/src/ComplexDatePicker.js (1 hunks)
- packages/@orchidui-vue/vite.config.js (2 hunks)
Additional comments: 2
packages/@orchidui-vue/src/ComplexDatePicker.js (1)
- 1-3: The changes correctly introduce a new import and export statement for
ComplexDatePicker. This centralizes the import path for consumers of the package, which is a good practice for maintainability and ease of use.packages/@orchidui-vue/vite.config.js (1)
- 48-52: The removal of
"@vueup/vue-quill"from the external dependencies list in therollupOptionssuggests that the package is either no longer used or is intended to be bundled with the library. Verify that this change aligns with the intended use of the package and that it won't cause issues with the library consumers who might expect this dependency to be external.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (1)
- packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (2 hunks)
Additional comments: 2
packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (2)
2-3: The import statement for
ComplexCalendarhas been updated to use a relative path instead of an absolute one. This change is likely made to ensure that the import is resolved correctly, especially if the package structure has changed or if there are issues with path resolution in the build setup. Ensure that the new relative path correctly points to theComplexCalendarcomponent's location within the project.42-48: The usage of
dayjsto format themodelValue.startandmodelValue.enddates within theInputcomponent is correct and follows best practices for date formatting in Vue templates. The use of thereadonlyattribute on theInputcomponent is also appropriate if the intention is to prevent users from manually editing the date.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (3)
- packages/@orchidui-vue/src/ComplexCalendar.js (1 hunks)
- packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (2 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (7 hunks)
Additional comments: 13
packages/@orchidui-vue/src/ComplexCalendar.js (1)
- 1-3: The changes correctly introduce a new import and export statement for the
ComplexCalendarcomponent, which aligns with the pull request's goal of enhancing modularity.packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (2)
2-3: The import statement for
ComplexCalendarhas been updated to use a relative path instead of an absolute one. This change should be verified to ensure that the correct component is being imported and that there are no issues with module resolution in the build process.44-46: The removal of
formatDatefromdayjs(modelValue.end)needs to be verified to ensure that it aligns with the intended functionality. If theformatDatefunction was previously used to format the date, its removal could lead to a change in how the date is displayed or processed. Ensure that this change is intentional and tested.packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (10)
1-8: The script setup block has been updated to remove TypeScript and use the Composition API setup style. This change should be verified to ensure that it aligns with the project's standards and that any TypeScript-specific features are not lost.
15-18: A new prop
imagehas been added. Ensure that this prop is documented and that its usage is clear to consumers of the component. Also, verify that the prop is being used correctly within the component.20-20: The
emitdefinition now includes "upload:image". This should be checked to ensure that the parent components are listening for this new event and handling it appropriately.22-24: The Quill Size attribute is being configured with a whitelist from
props.fontSizes. Ensure thatprops.fontSizesis always an array of objects with avalueproperty to avoid runtime errors.26-30: New refs have been added for
localValueand other states. Ensure that these refs are used consistently throughout the component and that their initial states are appropriate.38-46: The
base64Imagesref is initialized withprops.imageand is used in thereadImagefunction. Ensure that theprops.imageis in the correct format expected by the component and that there is proper validation for it.54-64: The
undoandredofunctions have been added to handle the respective actions. Ensure that these functions are tested and work as expected with the Quill editor instance.97-102: The
readImagefunction has been modified to updatebase64Imagesand emit "upload:image". Ensure that the emitted event is being handled correctly and that thebase64Imagesref is being used as intended.156-162: The
QuillEditorcomponent's:model-valueprop has been changed to:content. This change should be verified across the application to ensure that all instances ofQuillEditorare updated to use the new prop name.357-361: A new CSS file is imported, and styles are applied to
.ql-container. Verify that the imported CSS file exists and that the styles are correctly scoped to the component.Overall, the changes seem to aim at improving the functionality and maintainability of the
OcTextEditorcomponent. However, thorough testing and verification are required to ensure that the new features and changes integrate well with the existing system and that they do not introduce any regressions.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (1)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (7 hunks)
Additional comments: 7
packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (7)
1-8: The removal of the TypeScript
lang="ts"attribute from the script setup block is a significant change. Ensure that all TypeScript-specific code has been refactored to standard JavaScript and that no type-related features are lost in the transition.15-30: The addition of a new
imageprop and the correspondingupdate:imageevent emission is a new feature. Ensure that all parent components that useOcTextEditorare aware of and handle this new prop and event correctly.38-46: The
checkStatesfunction is tightly coupled with the Quill instance. Ensure that the Quill instance is always available when this function is called to avoid potential null reference errors.54-64: The
undoandredofunctions are using the Quill history stack. Ensure that the Quill history module is properly configured and that these functions are tested for correct behavior.97-102: The
readImagefunction now updates abase64Imagesref and emits anupdate:imageevent. Ensure that the emitted value is being used correctly and that thebase64Imagesref is managed properly throughout the component's lifecycle.156-162: The
QuillEditorcomponent's:contentprop is now bound tolocalValueinstead ofmodelValue. Ensure that this change is reflected in all places wheremodelValuewas previously used.357-361: The import of a local
snow.cssfile suggests a change in how styles are applied to the Quill editor. Verify that thesnow.cssfile is correctly scoped to the component and does not cause any unintended global style changes.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (1)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (6 hunks)
Additional comments: 6
packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (6)
1-8: Ensure that the removal of
@vueup/vue-quilland the local import ofQuillandQuillEditordoes not affect the functionality and that the local files provide all necessary features and compatibility.15-20: The addition of the
imageprop and the correspondingupdate:imageevent in theemitfunction is a significant change. Verify that all parent components that useOcTextEditorare updated to handle this new event.37-45: The
base64Imagesref is initialized with theprops.imagewhich implies that the image prop is expected to be a base64 string. Ensure that this is clearly documented and that any upstream code provides the image in the expected format.96-101: The
readImagefunction now updatesbase64Imagesand emits anupdate:imageevent. Ensure that the emitted event is being handled appropriately wherever this component is used.155-161: The
QuillEditorcomponent now bindscontentinstead ofmodelValue. This change should be verified for correctness and to ensure that it does not break two-way binding or the expected data flow in the application.357-360: The import of
snow.cssusing@importin the style block is a change in how styles are included. Verify that the styles are correctly applied and that there are no issues with style scoping or overrides.
…package-to-external
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- packages/@orchidui-vue/package.json
Files selected for processing (4)
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue (2 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (6 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1 hunks)
- packages/@orchidui-vue/src/composables/uploadFileProgress.js (2 hunks)
Files skipped from review due to trivial changes (2)
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue
- packages/@orchidui-vue/src/composables/uploadFileProgress.js
Additional comments: 5
packages/@orchidui-vue/src/Form/TextEditor/OcTextEditor.vue (5)
- 1-8: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [0-8]
The change from
lang="ts"tosetupin the script setup block is a Vue 3 specific feature that simplifies the composition API usage. Ensure that the rest of the component is compatible with this syntax.
15-20: The addition of the
imageprop and the correspondingupdate:imageevent emission is a new feature that allows for image handling within the text editor. Ensure that the parent components or any other components usingOcTextEditorare updated to handle this new prop and event.37-45: The initialization of
base64Imageswithprops.imageand the update withinreadImagefunction to emitupdate:imageis a good practice to keep the internal state consistent with the prop value. However, ensure that thebase64Imagesref is properly validated before use, especially since it's being used in an HTML context which could pose a security risk if not handled correctly (e.g., XSS attacks).155-165: The changes in the
QuillEditorcomponent props from:model-valueto:contentand:toolbarto:idshould be double-checked for compatibility with theQuillEditorcomponent's expected props. If these props were renamed in theQuillEditorcomponent, this change is correct. Otherwise, this could break the functionality.357-363: The import of
snow.cssis a good addition if thesnowtheme is being used for the Quill editor. Ensure that the CSS file is located at the correct path and is included in the build process.
| quill.value | ||
| .getQuill() | ||
| .clipboard.dangerouslyPasteHTML(range.index, `<img src="${base64}" />`); | ||
| base64Images.value = base64; | ||
| emit("update:image", base64Images.value); | ||
| }; |
There was a problem hiding this comment.
The readImage function directly sets the base64 image source into the editor's HTML. This could be a potential security risk if the base64 string is not properly sanitized. Ensure that the base64 string is validated against a known safe pattern before inserting it into the DOM.
| ? void 0 | ||
| : quill.setContents(normalizedContent, source); | ||
| } | ||
| internalModel = maybeClone(normalizedContent); | ||
| }; | ||
| const getText = (index, length) => { | ||
| var _a; | ||
| return (_a = | ||
| quill === null || quill === void 0 | ||
| ? void 0 | ||
| : quill.getText(index, length)) !== null && _a !== void 0 | ||
| ? _a | ||
| : ""; | ||
| }; | ||
| const setText = (text, source = "api") => { | ||
| quill === null || quill === void 0 ? void 0 : quill.setText(text, source); | ||
| }; | ||
| const getHTML = () => { | ||
| var _a; | ||
| return (_a = | ||
| quill === null || quill === void 0 ? void 0 : quill.root.innerHTML) !== | ||
| null && _a !== void 0 | ||
| ? _a | ||
| : ""; | ||
| }; | ||
| const setHTML = (html) => { | ||
| if (quill) quill.root.innerHTML = html; | ||
| }; | ||
| const pasteHTML = (html, source = "api") => { | ||
| const delta = | ||
| quill === null || quill === void 0 | ||
| ? void 0 | ||
| : quill.clipboard.convert(html); | ||
| if (delta) | ||
| quill === null || quill === void 0 | ||
| ? void 0 | ||
| : quill.setContents(delta, source); | ||
| }; | ||
| const focus = () => { | ||
| quill === null || quill === void 0 ? void 0 : quill.focus(); | ||
| }; | ||
| const reinit = () => { | ||
| nextTick(() => { | ||
| var _a; | ||
| if (!ctx.slots.toolbar && quill) | ||
| (_a = quill.getModule("toolbar")) === null || _a === void 0 | ||
| ? void 0 | ||
| : _a.container.remove(); | ||
| initialize(); | ||
| }); | ||
| }; | ||
| watch( | ||
| () => props.content, | ||
| (newContent) => { | ||
| if (!quill || !newContent || internalModelEquals(newContent)) return; | ||
| // Restore the selection and cursor position after updating the content | ||
| const selection = quill.getSelection(); | ||
| if (selection) { | ||
| nextTick(() => | ||
| quill === null || quill === void 0 | ||
| ? void 0 | ||
| : quill.setSelection(selection), | ||
| ); | ||
| } | ||
| setContents(newContent); | ||
| }, | ||
| { deep: true }, | ||
| ); | ||
| watch( | ||
| () => props.enable, | ||
| (newValue) => { | ||
| if (quill) quill.enable(newValue); | ||
| }, | ||
| ); | ||
| return { | ||
| editor, | ||
| getEditor, | ||
| getToolbar, | ||
| getQuill, | ||
| getContents, | ||
| setContents, | ||
| getHTML, | ||
| setHTML, | ||
| pasteHTML, | ||
| focus, | ||
| getText, | ||
| setText, | ||
| reinit, | ||
| }; | ||
| }, | ||
| render() { | ||
| var _a, _b; | ||
| return [ | ||
| (_b = (_a = this.$slots).toolbar) === null || _b === void 0 | ||
| ? void 0 | ||
| : _b.call(_a), | ||
| h("div", Object.assign({ ref: "editor" }, this.$attrs)), | ||
| ]; | ||
| }, | ||
| }); |
There was a problem hiding this comment.
The QuillEditor Vue component is well-structured and provides a comprehensive set of features for integrating the Quill rich text editor. Here are some points to consider:
- The
idprop's validator (lines 46-54) checks if the value starts with a#, which is typically a selector id in CSS. Ensure that this is the intended use case and that consumers of this component are aware of this requirement. - The
registerModulefunction (lines 89-96) checks if the module is already registered but does not provide feedback if the registration is skipped. Consider logging a warning or providing a callback for this case. - The
initializefunction (lines 99-131) sets up the Quill editor and registers modules. Ensure that the editor's container is properly referenced and that the editor is not initialized more than once ifinitializeis called multiple times. - The
composeOptionsfunction (lines 134-172) merges various options for the Quill editor. It's important to ensure that the merging ofprops.globalOptions,props.options, andclientOptionsis done in the correct order to maintain the expected precedence of option settings. - The
handleTextChangefunction (lines 201-207) emits an update for thecontentprop. Ensure that this does not lead to any unintended side-effects, especially in the context of Vue's reactivity system. - The
setContentsfunction (lines 266-281) normalizes the content based on thecontentTypeprop. Ensure that the normalization process is robust and handles all edge cases. - The
reinitfunction (lines 319-326) is designed to reinitialize the editor. This could be potentially destructive if not used carefully. Ensure that there is clear documentation on when and how to use this function. - The
watchfunctions (lines 329-345 and 346-351) are set up to react to changes inprops.contentandprops.enable. Ensure that these watchers do not introduce performance issues, especially with the deep watcher forprops.content. - The
renderfunction (lines 368-376) uses slots for toolbar customization and sets up the editor'sdiv. Ensure that the slots are properly documented and that therefis correctly assigned to the editor'sdiv.
Overall, the component seems to be well thought out, but it's crucial to ensure that all edge cases and potential reactivity issues are considered. Additionally, proper documentation for the component's usage, especially for the more complex features like module registration and editor reinitialization, would be beneficial for maintainers and consumers of the component.
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (11)
- packages/@orchidui-vue/src/Builder/DataTable/OcDataTable.vue (1 hunks)
- packages/@orchidui-vue/src/DataDisplay/Table/OcTableCell.vue (1 hunks)
- packages/@orchidui-vue/src/Disclosure/Tabs/OcTabs.vue (1 hunks)
- packages/@orchidui-vue/src/Form/Checkbox/OcCheckbox.vue (1 hunks)
- packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (1 hunks)
- packages/@orchidui-vue/src/Form/SingleFileUpload/ModalCropper.vue (1 hunks)
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1 hunks)
- packages/@orchidui-vue/src/Form/TextEditor/snow.css (1 hunks)
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePicker.vue (1 hunks)
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePopup.vue (1 hunks)
Files skipped from review due to trivial changes (7)
- packages/@orchidui-vue/src/Builder/DataTable/OcDataTable.vue
- packages/@orchidui-vue/src/DataDisplay/Table/OcTableCell.vue
- packages/@orchidui-vue/src/Disclosure/Tabs/OcTabs.vue
- packages/@orchidui-vue/src/Form/SingleFileUpload/ModalCropper.vue
- packages/@orchidui-vue/src/Form/SingleFileUpload/SingleFileUpload.vue
- packages/@orchidui-vue/src/Form/TextEditor/snow.css
- packages/@orchidui-vue/src/Form/TimePicker/OcTimePicker.vue
Additional comments: 2
packages/@orchidui-vue/src/Form/ComplexDatePicker/OcComplexDatePicker.vue (1)
- 3-3: The import statement for
ComplexCalendarhas been updated to reflect the new location of theOcComplexCalendarcomponent. Ensure that the new path is correct and that the component is correctly exported from the new location to prevent any import errors.packages/@orchidui-vue/src/Form/TextEditor/QuillEditor.js (1)
- 1-377: The introduction of the
QuillEditorcomponent is a significant addition to the@orchidui-vuepackage. The component is well-structured, with clear prop definitions, event emissions, and methods for interacting with the Quill editor. Here are some points to consider:
- Ensure that the
Quilllibrary is included in the package's dependencies and is properly installed.- The
idprop's validator (lines 46-54) checks if the value starts with a#, which is typically a selector id convention in CSS. Verify that this is the intended use case and that it aligns with how theidprop is used within the component.- The
registerModulefunction (lines 89-96) checks if the module is already registered before attempting to register it. This is a good practice to avoid unnecessary registrations.- The
initializefunction (lines 99-131) sets up the Quill instance and registers event handlers. It also handles the toolbar's mousedown event to prevent it from triggering a blur event on the editor, which is a thoughtful touch.- The
composeOptionsfunction (lines 134-172) merges various options objects to configure the Quill editor. It's important to ensure that the merging order doesn't unintentionally override important settings.- The
handleTextChange,handleSelectionChange, andhandleEditorChangemethods (lines 201-235) emit corresponding events, which allows parent components to react to changes in the editor state.- The
getContents,setContents,getText,setText,getHTML,setHTML, andpasteHTMLmethods (lines 256-315) provide a comprehensive API for interacting with the editor's content in different formats.- The
focusandreinitmethods (lines 316-328) offer additional control over the editor instance, such as focusing the editor and reinitializing it.- The
watchfunctions (lines 329-351) are used to reactively handle changes to thecontentandenableprops, ensuring the editor updates accordingly.- The
renderfunction (lines 368-376) returns the toolbar slot and the editor div, which is the standard pattern for Vue components.Overall, the component seems to be well-implemented with attention to detail. However, it's crucial to test the component thoroughly, especially the reactive prop handling and event emissions, to ensure it behaves as expected in various scenarios.
| ? 0 | ||
| : hour.value | ||
| : hour.value === 12 | ||
| ? 12 | ||
| : hour.value + 12, | ||
| ? 12 | ||
| : hour.value + 12, | ||
| minutes.value - 1, | ||
| 59, | ||
| 0, |
There was a problem hiding this comment.
The logic for setting the hour value in the Date object seems correct, but it's important to ensure that the minutes.value is correctly adjusted. The current logic subtracts 1 from the minutes.value, which may not be intended. If the goal is to set the minutes to the exact value selected by the user, this subtraction should be removed.
- minutes.value - 1,
+ minutes.value,Commitable suggestion
[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.
| ? 0 | |
| : hour.value | |
| : hour.value === 12 | |
| ? 12 | |
| : hour.value + 12, | |
| ? 12 | |
| : hour.value + 12, | |
| minutes.value - 1, | |
| 59, | |
| 0, | |
| ? 0 | |
| : hour.value | |
| : hour.value === 12 | |
| ? 12 | |
| : hour.value + 12, | |
| minutes.value, | |
| 59, | |
| 0, |
| ? 'border-oc-primary-100 bg-oc-primary-100' | ||
| : 'border-oc-primary bg-oc-primary' | ||
| : isError | ||
| ? 'border-oc-error' | ||
| : isDisabled | ||
| ? 'bg-oc-primary-50 border-oc-primary-200' | ||
| : 'border-oc-primary-200', | ||
| ? 'border-oc-error' | ||
| : isDisabled | ||
| ? 'bg-oc-primary-50 border-oc-primary-200' | ||
| : 'border-oc-primary-200', | ||
| isError && modelValue && !isDisabled ? '!bg-oc-error ' : '', | ||
| isError && !isDisabled ? '!border-oc-error' : '', | ||
| isPartial |
There was a problem hiding this comment.
The refactoring of the class bindings improves readability by separating the conditions more clearly. However, ensure that the logic for class assignment is thoroughly tested, especially since the use of !important in class names (!bg-oc-error, !border-oc-error) can lead to CSS specificity issues that might override other styles unintentionally. It's generally best to avoid !important where possible and rely on a well-structured CSS specificity hierarchy.
- isError && modelValue && !isDisabled ? '!bg-oc-error ' : '',
- isError && !isDisabled ? '!border-oc-error' : '',
- isError && modelValue && !isDisabled ? 'bg-oc-error' : '',
- isError && !isDisabled ? 'border-oc-error' : '',
If the use of `!important` is necessary due to the specificity issues that cannot be resolved otherwise, the current change is acceptable. However, if it's possible to refactor the CSS to avoid `!important`, that would be preferable for maintainability and to avoid potential conflicts in the future.
<!-- This is an auto-generated comment by CodeRabbit -->
Summary by CodeRabbit
New Features
OcComplexCalendarandOcComplexDatePickercomponents for enhanced date selection.QuillEditorcomponent for rich text editing with various toolbar options.OcSingleFileUploadwith a modal cropper for image uploads.Enhancements
OcTextEditorto support image uploads and improved content handling.OcTimePickerandOcTimePopupcomponents for better time selection.OcDataTableandOcTabswith refined conditional class assignments.OcCheckboxwith updated class application logic.Style Updates
QuillEditortext editor component.Refactor
Bug Fixes
OcTableCellandOcTabs.OcTimePopup.Documentation
Chores