-
Notifications
You must be signed in to change notification settings - Fork 340
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor:plugin adjusts to the left and right sides #785
refactor:plugin adjusts to the left and right sides #785
Conversation
WalkthroughThe changes across multiple components focus on enhancing styling and layout management by introducing new computed properties for alignment and margin calculations. The Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PluginSetting
participant LayoutManager
User->>PluginSetting: Set alignment
PluginSetting->>LayoutManager: Get alignment info
LayoutManager-->>PluginSetting: Return alignment
PluginSetting->>PluginSetting: Compute style based on alignment
PluginSetting->>User: Render updated layout
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- packages/common/component/PluginSetting.vue (5 hunks)
- packages/design-core/config/addons.js (2 hunks)
- packages/design-core/config/plugin.js (1 hunks)
- packages/design-core/src/App.vue (1 hunks)
- packages/plugins/bridge/src/BridgeSetting.vue (3 hunks)
- packages/plugins/data/src/Main.vue (4 hunks)
- packages/plugins/datasource/src/DataSourceForm.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceRecordList.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceRemotePanel.vue (3 hunks)
- packages/plugins/page/src/PageFolderSetting.vue (3 hunks)
- packages/plugins/page/src/PageSetting.vue (3 hunks)
Additional comments not posted (44)
packages/design-core/config/plugin.js (1)
Line range hint
1-11
: Verify the impact of removingaddons.settings
and consider merging it intoaddons.plugins
.The removal of the loop that populated the
plugin
object with entries fromaddons.settings
can potentially affect any functionality that relied on those settings being included in theplugin
object. ThegetPlugin
function will no longer be able to return plugins defined inaddons.settings
.Please verify if the removal of
addons.settings
is intentional and if it affects any functionality. You can run the following script to search for any usage ofaddons.settings
in the codebase:If the settings from
addons.settings
are still required, consider mergingaddons.settings
intoaddons.plugins
to maintain the functionality. You can apply this diff to merge the settings:import addons from './addons' const plugin = {} -addons.plugins.forEach((item) => { +[...addons.plugins, ...addons.settings].forEach((item) => { plugin[item.id] = item }) export const getPlugin = (pluginName) => { return plugin[pluginName] || null }Verification successful
Removal of
addons.settings
is intentional and does not affect functionality.The search results confirm that
addons.settings
is not defined or used anywhere else in the codebase, indicating that its removal from theplugin
object is intentional and does not impact any other functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any usage of `addons.settings` in the codebase. # Test: Search for `addons.settings` usage. Expect: No occurrences if the removal is intentional and doesn't affect functionality. rg --type js $'addons\.settings'Length of output: 33
Script:
#!/bin/bash # Description: Search for the definition of `addons.settings` in the codebase. # Test: Search for `addons.settings` definition. Expect: No occurrences if it is obsolete or unused. rg --type js $'settings' --context 5Length of output: 14347
packages/design-core/config/addons.js (2)
48-64
: LGTM!The changes to the
plugins
array are approved:
- 3 new plugins (
Props
,Styles
, andEvents
) have been correctly added to the array.- The new plugins are correctly imported from the
@opentiny/tiny-engine-setting-*
packages.- The change is consistent with the PR summary.
80-80
: Verify the impact of removing thesettings
array.The removal of the
settings
array is approved as it is consistent with the PR summary and the AI-generated summary.However, please verify that the removal of the
settings
array does not break any existing functionality in the application. Run the following script to search for any potential usage of thesettings
array:Verification successful
Removal of the
settings
array is safe and verified.The search did not reveal any direct usage of
addons.settings
, confirming that its removal does not impact existing functionality. The change is consistent with the PR summary and does not introduce any issues.
- No direct references to
addons.settings
were found in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any potential usage of the `settings` array. # Test: Search for the `settings` array usage. Expect: No results. rg --type js $'addons.settings'Length of output: 30
Script:
#!/bin/bash # Description: Search for any potential usage of the `settings` array in the codebase. # Test: Search for any reference to `settings` in JavaScript files. rg --type js 'settings'Length of output: 2549
packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue (3)
3-3
: LGTM!The changes to the
plugin-setting
component's attributes improve readability and introduce a more flexible styling approach based on the plugin's alignment.
51-58
: LGTM!The refactoring of the computed properties improves the logic for styling the component, making it more adaptable to different layout configurations. The introduction of the
align
andmargin
properties, along with thecomputedStyle
property, enhances the control flow related to styling and layout management.
102-103
: LGTM!The changes to the returned object of the setup function are consistent with the refactoring of the computed properties. The inclusion of
computedStyle
andalign
properties aligns with the updated styling logic.packages/design-core/src/App.vue (1)
Line range hint
72-78
: Verify the impact of the change and consider adding safety checks and default values.The change in the iteration from
addons.settings
toaddons.plugins
suggests a shift in how theplugin
object is constructed. Please verify that this change does not introduce any unintended consequences or break any existing functionality.Consider adding a check for the existence of
item.id
before setting theplugin
object to avoid unexpected behavior:addons.plugins.forEach((item) => { + if (item.id) { plugin[item.id] = { width: item.options?.width || 300, align: item.options?.align || 'rightTop' } + } })Also, consider setting default values for
width
andalign
based onitem.options
to maintain consistency:addons.plugins.forEach((item) => { if (item.id) { - plugin[item.id] = { width: item.options?.width || 300, align: item.options?.align || 'rightTop' } + plugin[item.id] = { width: item.options?.width || 300, align: item.options?.align || 'rightTop' } } })Lastly, is it necessary to store the
plugin
object in the local storage? If not, consider removing this line:- localStorage.setItem('plugin', JSON.stringify(plugin))
packages/common/component/PluginSetting.vue (6)
3-3
: LGTM!The code changes are approved.
4-4
: LGTM!The code changes are approved.
34-34
: LGTM!The code changes are approved.
94-96
: LGTM!The code changes are approved.
118-124
: LGTM!The code changes are approved.
151-159
: LGTM!The code changes are approved.
packages/plugins/page/src/PageFolderSetting.vue (6)
6-6
: LGTM!The change to use the
computedStyle
computed property for the:style
binding is approved. This enhances the flexibility of the component by allowing it to adapt its styling based on the layout configuration.
7-7
: LGTM!The change to compute the
:align
property using thegetPluginByLayout
function is approved. This determines the alignment of the plugin based on its layout, which is a good improvement.
86-88
: LGTM!The changes to import new constants and introduce new computed properties are approved. These changes are part of the refactoring to enhance the styling and layout management.
89-91
: LGTM!The introduction of the
computedStyle
computed property is approved. This change improves the readability and maintainability of the code by consolidating style calculations into a single computed property.
215-215
: LGTM!The change to return the
computedStyle
computed property from thesetup
function is approved. This is necessary to make thecomputedStyle
computed property available in the template.
216-216
: LGTM!The change to return the
align
computed property from thesetup
function is approved. This is necessary to make thealign
computed property available in the template.packages/plugins/datasource/src/DataSourceForm.vue (4)
2-2
: LGTM!The changes to the
plugin-setting
component's bindings are part of the refactoring to enhance the styling and layout management. The usage of computed propertiescomputedStyle
andalign
improves the adaptability of the component's layout based on the plugin's configuration.
86-88
: LGTM!The introduction of the computed properties
align
andmargin
is part of the refactoring to improve the layout management.align
determines the alignment of the plugin based on its configuration, whilemargin
retrieves the width of the plugin. These properties are used to dynamically calculate the styling in thecomputedStyle
property.
89-91
: LGTM!The
computedStyle
property dynamically calculates the margin based on the alignment of the plugin. This change improves the responsiveness and adaptability of the component's layout, allowing for better handling of different alignment scenarios.
263-264
: LGTM!The changes to the exported properties reflect the refactoring of the layout management.
computedStyle
andalign
are now part of the exported object, replacing the previousleftMargin
property. This ensures that the component's layout is properly managed based on the plugin's configuration.packages/plugins/datasource/src/DataSourceRemotePanel.vue (5)
8-9
: LGTM!The changes to use the new
computedStyle
andalign
computed properties for dynamically determining the margin and alignment of the panel based on the plugin's layout configuration are approved.
105-105
: LGTM!The change to import the
getPluginByLayout
method from theuseLayout
composable is approved.
107-112
: LGTM!The changes to introduce the new
align
,margin
, andcomputedStyle
computed properties for dynamically determining the margin and alignment of the panel based on the plugin's layout configuration are approved.
225-225
: LGTM!The change to return the
computedStyle
computed property from thesetup
function is approved.
226-226
: LGTM!The change to return the
align
computed property from thesetup
function is approved.packages/plugins/bridge/src/BridgeSetting.vue (4)
2-2
: LGTM!The changes to the
style
binding and the addition of thealign
property enhance the flexibility and adaptability of the component's layout.
156-162
: LGTM!The introduction of the new computed properties
align
,margin
, andcomputedStyle
improves the responsiveness and adaptability of the component's layout. The computed properties are well-structured and follow a logical flow.
318-318
: LGTM!Returning the
computedStyle
computed property from thesetup
function allows it to be used in the template.
319-319
: LGTM!Returning the
align
computed property from thesetup
function allows it to be used in the template.packages/plugins/page/src/PageSetting.vue (4)
2-2
: LGTM!The changes to the
plugin-setting
component's attributes improve readability and introduce dynamic layout management through the newcomputedStyle
andalign
properties.
126-127
: LGTM!The newly imported
getPluginWidth
andgetPluginByLayout
properties fromuseLayout
are necessary for the subsequent code changes that compute the plugin's alignment and margin.
128-132
: LGTM!The new computed properties,
align
,margin
, andcomputedStyle
, work together to dynamically adjust the plugin's margin based on its alignment, enhancing the layout's flexibility and responsiveness.
380-381
: LGTM!The addition of
computedStyle
andalign
properties to the exported object is consistent with the previous code changes and reflects the updated public API of the component.packages/plugins/data/src/Main.vue (4)
Line range hint
44-144
: LGTM!The code changes are approved. The introduction of the
computedStyle
computed property enhances the styling and layout management of the right panel.
130-130
: LGTM!The code changes are approved. The addition of the
getPluginByLayout
function to the destructured properties is consistent with the PR objectives and the AI-generated summary.
136-144
: LGTM!The code changes are approved. The introduction of the
align
,margin
, andcomputedStyle
computed properties enhances the adaptability of the right panel's layout, making it more responsive to the plugin's alignment and margin.
408-409
: LGTM!The code changes are approved. The addition of
computedStyle
andalign
to the returned object of thesetup
function, replacing theleftMargin
property, is consistent with the PR objectives and the AI-generated summary.packages/plugins/datasource/src/DataSourceRecordList.vue (4)
8-9
: LGTM!The changes introduce new computed properties to dynamically set the style and alignment of the component. This improves the flexibility and responsiveness of the component to layout changes.
141-142
: LGTM!The changes introduce new utility functions from the
useLayout
hook to get the plugin width and layout information. This information is used to compute the style and alignment of the component.
143-148
: LGTM!The changes introduce new computed properties to dynamically set the style and alignment of the component based on the layout information. This improves the flexibility and responsiveness of the component to layout changes.
609-610
: LGTM!The changes expose the new computed properties to the template, allowing them to be used to dynamically set the style and alignment of the component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- packages/common/component/PluginSetting.vue (6 hunks)
- packages/plugins/bridge/src/BridgeSetting.vue (3 hunks)
- packages/plugins/data/src/Main.vue (4 hunks)
- packages/plugins/datasource/src/DataSourceForm.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceRecordList.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceRemotePanel.vue (3 hunks)
- packages/plugins/page/src/PageFolderSetting.vue (3 hunks)
- packages/plugins/page/src/PageSetting.vue (3 hunks)
Files skipped from review as they are similar to previous changes (8)
- packages/common/component/PluginSetting.vue
- packages/plugins/bridge/src/BridgeSetting.vue
- packages/plugins/data/src/Main.vue
- packages/plugins/datasource/src/DataSourceForm.vue
- packages/plugins/datasource/src/DataSourceRecordList.vue
- packages/plugins/datasource/src/DataSourceRemotePanel.vue
- packages/plugins/page/src/PageFolderSetting.vue
- packages/plugins/page/src/PageSetting.vue
Additional comments not posted (3)
packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue (3)
3-3
: Refactor: Simplified template with dynamic styling.The template now uses
computedStyle
for dynamic margin adjustments andalign
for alignment, enhancing flexibility and readability. This change aligns with the PR's objective to make plugins adaptable to both sides of the interface.
51-58
: Enhanced layout adaptability with new computed properties.The introduction of
align
,margin
, andcomputedStyle
computed properties in the setup function enhances the component's adaptability to different layout configurations. This change supports dynamic styling based on the plugin's position, which is a key objective of the PR.
102-103
: Ensure proper export of new computed properties.The
computedStyle
andalign
properties are now part of the component's return object, ensuring they are accessible in the template. This is crucial for the dynamic styling functionality introduced in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@STATICHIT commit没有签名,需要改下
25539ce
to
51f55ab
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
packages/design-core/config/plugin.js (1)
Line range hint
1-5
: Consider adding error handling for plugin IDs.The current implementation assumes that each plugin
id
is unique and present. Consider adding error handling to manage potential issues with duplicate or missingid
fields.packages/plugins/bridge/src/BridgeSetting.vue (1)
156-162
: Approve the implementation of computed properties.The implementation of
align
,margin
, andcomputedStyle
is effective for dynamic styling based on the plugin's layout. Consider adding comments to explain the logic behind these computed properties for better maintainability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- packages/common/component/PluginSetting.vue (6 hunks)
- packages/design-core/config/addons.js (2 hunks)
- packages/design-core/config/plugin.js (1 hunks)
- packages/design-core/src/App.vue (1 hunks)
- packages/plugins/bridge/src/BridgeSetting.vue (3 hunks)
- packages/plugins/data/src/Main.vue (4 hunks)
- packages/plugins/datasource/src/DataSourceForm.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceRecordList.vue (3 hunks)
- packages/plugins/datasource/src/DataSourceRemotePanel.vue (3 hunks)
- packages/plugins/page/src/PageFolderSetting.vue (3 hunks)
- packages/plugins/page/src/PageSetting.vue (3 hunks)
Files skipped from review as they are similar to previous changes (10)
- packages/common/component/PluginSetting.vue
- packages/design-core/config/addons.js
- packages/design-core/src/App.vue
- packages/plugins/data/src/Main.vue
- packages/plugins/datasource/src/DataSourceForm.vue
- packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue
- packages/plugins/datasource/src/DataSourceRecordList.vue
- packages/plugins/datasource/src/DataSourceRemotePanel.vue
- packages/plugins/page/src/PageFolderSetting.vue
- packages/plugins/page/src/PageSetting.vue
Additional comments not posted (2)
packages/design-core/config/plugin.js (1)
6-8
: Verify the impact of removingaddons.settings
.The
getPlugin
function now only retrieves plugins fromaddons.plugins
. Verify that this change does not negatively impact other parts of the application that might have relied onaddons.settings
.Verification successful
No impact from removing
addons.settings
.The search confirmed that there are no references to
addons.settings
in the codebase, indicating that its removal does not affect other parts of the application. The change to thegetPlugin
function is isolated and safe.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if any part of the application still references `addons.settings`. # Test: Search for references to `addons.settings`. Expect: No occurrences. rg --type javascript -A 5 $'addons.settings'Length of output: 78
Script:
#!/bin/bash # Description: Verify if any part of the application still references `addons.settings`. # Test: Search for references to `addons.settings`. Expect: No occurrences. rg --type js -A 5 $'addons.settings'Length of output: 35
packages/plugins/bridge/src/BridgeSetting.vue (1)
Line range hint
156-318
: Verify the initialization and usage of reactive properties.Ensure that all reactive properties are properly initialized and used throughout the component to prevent any issues related to reactivity or data flow.
Verification successful
Reactive Properties in BridgeSetting.vue are Properly Initialized and Used
The
state
object inBridgeSetting.vue
is correctly initialized usingreactive
and is consistently used throughout the component. The references in the template and logic confirm its proper integration with Vue's reactivity system. No issues were found with its initialization or usage.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the initialization and usage of reactive properties in the Vue component. # Test: Search for reactive property usage. Expect: Proper initialization and consistent usage. rg --type vue -A 5 $'reactive'Length of output: 57
Script:
#!/bin/bash # Description: Verify the initialization and usage of reactive properties in the Vue component using ast-grep. # Test: Search for reactive property usage in Vue files. ast-grep --lang vue --pattern 'reactive({$$$})'Length of output: 156
Script:
#!/bin/bash # Description: Verify the initialization and usage of reactive properties in the Vue component using rg. # Test: Search for reactive property usage in the codebase. rg 'reactive\('Length of output: 17618
Script:
#!/bin/bash # Description: Verify the usage of the 'state' object in the BridgeSetting.vue component. # Test: Search for references to 'state' in the BridgeSetting.vue file. rg 'state\.' packages/plugins/bridge/src/BridgeSetting.vueLength of output: 2284
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
1.将addons中的setting数组合并到plugins中,并处理项目相关处代码
2.所有带二级面板的插件根据插件所在位置判断显示位置
3.所有插件根据所在位置进行阴影适应
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
New Features
align
property in thePluginSetting
component for enhanced alignment control.Improvements
Bug Fixes