-
Notifications
You must be signed in to change notification settings - Fork 438
Feat/resources: Add Resource Manager Plugin & Select Source Configurator #1624
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
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a Resource plugin and SourceSelectConfigurator, updates Img schema to use the new selector, stitches the plugin into design-core/layout/register/dev-alias, provides plugin UI and HTTP APIs, and wires optional Tailwind script injection into canvas/preview flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant ConfigField as Img Config Field
participant SrcSel as SourceSelectConfigurator
participant API as Resource API (meta)
U->>ConfigField: Click “资源选择”
ConfigField->>SrcSel: openPopover()
SrcSel->>API: fetchResourceGroupByAppId(appId)
API-->>SrcSel: categories + resources
U->>SrcSel: filter/search/select resource
U->>SrcSel: Confirm
SrcSel-->>ConfigField: emit update:modelValue(resourceUrl)
ConfigField-->>U: img.src updated
sequenceDiagram
autonumber
participant U as User
participant Plugin as Resource Plugin (Main)
participant List as ResourceList
participant Setting as ResourceSetting
participant API as Resource HTTP
Note over Plugin: mount → load groups
Plugin->>API: fetchResourceGroupByAppId()
API-->>Plugin: groups
U->>Plugin: Open group
Plugin->>List: openResourceListPanel(group)
List->>API: fetchResourceListByGroupId(group.id)
API-->>List: resources
U->>List: Upload/URL add
List->>API: batchCreateResource(...)
API-->>List: OK
List-->>Plugin: refresh groups
U->>Plugin: Open group settings
Plugin->>Setting: openResourceSettingPanel(data)
U->>Setting: Save
alt edit
Setting->>API: updateResourceGroup(id, data)
else create
Setting->>API: createResourceGroup(data)
end
API-->>Setting: OK
Setting-->>Plugin: emit create-group
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (107)
⛔ Files not processed due to max files limit (20)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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: 17
🧹 Nitpick comments (20)
packages/plugins/resource/vite.config.ts (1)
34-35: Tiny nit: terminate the injected import.
Prevents occasional tooling warnings.- banner: 'import "./style.css"' + banner: 'import "./style.css";'packages/plugins/resource/src/js/http.ts (3)
26-31: Avoid sending undefined platformId.
Only include platformId when present.-export const createResource = (params: any) => - getMetaApi(META_SERVICE.Http).post(`${baseUrl}/resource/create`, { - appId: getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id, - platformId: getMergeMeta('engine.config')?.platformId, - ...params - }) +export const createResource = (params: any) => { + const appId = getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id + const platformId = getMergeMeta('engine.config')?.platformId + const body = { appId, ...(platformId != null ? { platformId } : {}), ...params } + return getMetaApi(META_SERVICE.Http).post(`${baseUrl}/resource/create`, body) +}
60-65: Same conditional platformId pattern for group creation.-export const createResourceGroup = (params: any) => - getMetaApi(META_SERVICE.Http).post(`${baseUrl}/resource-group/create`, { - appId: getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id, - platformId: getMergeMeta('engine.config')?.platformId, - ...params - }) +export const createResourceGroup = (params: any) => { + const appId = getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id + const platformId = getMergeMeta('engine.config')?.platformId + const body = { appId, ...(platformId != null ? { platformId } : {}), ...params } + return getMetaApi(META_SERVICE.Http).post(`${baseUrl}/resource-group/create`, body) +}
54-57: Unify API with configurator’s variant.
Expose optional appId for flexibility and parity with configurator/http.ts.-export const fetchResourceGroupByAppId = () => - getMetaApi(META_SERVICE.Http).get( - `${baseUrl}/resource-group/${getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id}` - ) +export const fetchResourceGroupByAppId = (appId?: number) => + getMetaApi(META_SERVICE.Http).get( + `${baseUrl}/resource-group/${appId ?? getMetaApi(META_SERVICE.GlobalService).getBaseInfo().id}` + )packages/configurator/src/index.ts (1)
73-105: Consider adding Meta-prefixed alias for backward compatibility.
Matches existing pattern for legacy names.MetaCodeEditor as CodeConfigurator, // 暂时保留Meta开头的旧组件名 ArrayItemConfigurator as MetaArrayItem, + SourceSelectConfigurator as MetaSourceSelect, I18nInput as MetaBindI18n,designer-demo/public/mock/bundle.json (2)
4436-4439: Label/description wording consistency.Label "资源选择" + description "src路径" reads slightly redundant. Consider unifying across similar fields (e.g., video.src) for consistent UX wording.
4428-4450: Add accessibility support (alt text).Consider adding an
altprop in this schema to let users set alternative text for images."content": [ { "property": "src", ... "labelPosition": "top" }, + { + "property": "alt", + "type": "string", + "label": { "text": { "zh_CN": "替代文本" } }, + "cols": 12, + "widget": { "component": "InputConfigurator", "props": {} }, + "description": { "zh_CN": "图片无法显示时的文本,有助于无障碍访问" }, + "labelPosition": "left" + },packages/plugins/resource/package.json (3)
10-12: Add exports/types for better ESM/TS consumption.Expose explicit exports and generated typings to avoid resolution issues in downstream builds.
Apply:
"type": "module", - "main": "dist/index.js", - "module": "dist/index.js", + "main": "dist/index.js", + "module": "dist/index.js", + "types": "dist/index.d.ts", + "exports": { + ".": { + "types": "./dist/index.d.ts", + "import": "./dist/index.js", + "default": "./dist/index.js" + } + },
13-15: Consider marking package as side-effect free.Helps tree-shaking; safe if no top-level side effects.
Apply:
"files": [ "dist" ], + "sideEffects": false,
38-42: Align peer dependency ranges across the monorepo.
- ./packages/design-core/package.json and ./packages/layout/package.json declare "vue": "^3.4.23" while most packages declare "vue": "^3.4.15".
- Several packages omit "@opentiny/vue-icon" (examples: packages/plugins/script, packages/plugins/tutorial, packages/settings/design, many toolbars) while most that include it use "^3.20.0".
- Action: pick a canonical set (e.g., @opentiny/*: ^3.20.0 and vue: ^3.4.15, or upgrade all to ^3.4.23) and update package.json files to match to avoid peer conflicts.
packages/configurator/src/source-select-configurator/http.ts (1)
16-22: DRY: reuse the plugin’s HTTP helpers to avoid endpoint drift.These functions duplicate ones in packages/plugins/resource/src/js/http.ts. Consider re-exporting from a shared module (e.g., tiny-engine-common) to keep a single source of truth.
packages/plugins/resource/src/ResourceList.vue (4)
213-214: Type-safe copy state to ensure popover toggling works.Initialize as null and compare strictly; current init to '' can cause type mismatches.
Apply:
- const copySourceId = ref('') + const copySourceId = ref(null)- :modelValue="item.id && item.id === copySourceId" + :modelValue="copySourceId !== null && item.id === copySourceId"- copySourceId.value = item.id + copySourceId.value = item.id @@ - copySourceId.value = '' + copySourceId.value = nullAlso applies to: 52-56
209-209: Implement the type filter or remove the toggle.
sourceTypeis never applied; wire a filtered list to the v-for.Apply:
- const sourceType = ref('all') + const sourceType = ref('all') + const filteredSourceList = computed(() => + sourceType.value === 'all' ? state.sourceList : state.sourceList.filter((s) => s.category === 'image') + )- <div v-for="item in state.sourceList" :key="item.id" class="source-list-item"> + <div v-for="item in filteredSourceList" :key="item.id" class="source-list-item">return { @@ - sourceType, + sourceType, + filteredSourceList,Also applies to: 462-489, 36-44
176-183: Ensure list loads on first open and when reopening same group.If the same group is reopened, the watcher may not fire. Trigger fetch on open.
Apply:
export const openResourceListPanel = (group) => { state.group = { ...group } isShow.value = true + getSourceList() }Optionally keep the watcher for subsequent id changes.
Also applies to: 455-460
504-512: Cursor pointer on the whole action bar may be misleading.Limit pointer to clickable items to improve UX.
Apply:
- cursor: pointer; + .resource-type-active, + .batch-action, + .action-item { cursor: pointer; }packages/plugins/resource/src/ResourceSetting.vue (1)
76-76: Remove empty props declarationThe component has an empty
propsdeclaration which is unnecessary when no props are defined.- props: {},packages/plugins/resource/src/Main.vue (2)
85-93: Optimize active state managementThe current implementation loops through all items to set active state. Consider using a more efficient approach with a single active ID reference.
+const activeItemId = ref(null) + const setItemActive = (data) => { - resourceList.value.forEach((item) => { - if (data.id === item.id) { - item.active = true - } else { - item.active = false - } - }) + activeItemId.value = data.id }Then update the template to use the computed active state:
- :class="['resource-item', { 'active-item': item.active }]" + :class="['resource-item', { 'active-item': item.id === activeItemId }]"
122-124: Remove unused updateCategory functionThe
updateCategoryfunction is defined but never used in the component. It's also exposed in the return statement but not called anywhere.- const updateCategory = (data) => { - resourceList.value = data - }And remove it from the return statement:
openResourceList, createCategory, - updateCategorypackages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue (2)
173-178: Add case-insensitive searchThe current search implementation is case-sensitive. Consider making it case-insensitive for better user experience.
watch( () => searchWords.value, () => { - sourceList.value = sourceOriginList.value.filter((item) => item.name.includes(searchWords.value)) + const searchTerm = searchWords.value.toLowerCase() + sourceList.value = sourceOriginList.value.filter((item) => + item.name.toLowerCase().includes(searchTerm) + ) } )Also update line 156:
- sourceList.value = sourceOriginList.value.filter((item) => item.name.includes(searchWords.value)) + const searchTerm = searchWords.value.toLowerCase() + sourceList.value = sourceOriginList.value.filter((item) => + item.name.toLowerCase().includes(searchTerm) + )
49-49: Use nullish coalescing operator correctlyThe nullish coalescing operator should be written as
??not??.- <img :src="item.thumbnailUrl ?? item.resourceUrl" /> + <img :src="item.thumbnailUrl || item.resourceUrl" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/design-core/assets/plugin-icon-resource.svgis excluded by!**/*.svg
📒 Files selected for processing (19)
designer-demo/public/mock/bundle.json(1 hunks)packages/build/vite-config/src/vite-plugins/devAliasPlugin.js(1 hunks)packages/canvas/render/src/builtin/builtin.json(1 hunks)packages/configurator/src/index.ts(2 hunks)packages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue(1 hunks)packages/configurator/src/source-select-configurator/http.ts(1 hunks)packages/design-core/package.json(1 hunks)packages/design-core/re-export.js(1 hunks)packages/design-core/registry.js(2 hunks)packages/layout/src/defaultLayout.js(1 hunks)packages/plugins/resource/index.ts(1 hunks)packages/plugins/resource/meta.js(1 hunks)packages/plugins/resource/package.json(1 hunks)packages/plugins/resource/src/Main.vue(1 hunks)packages/plugins/resource/src/ResourceList.vue(1 hunks)packages/plugins/resource/src/ResourceSetting.vue(1 hunks)packages/plugins/resource/src/js/http.ts(1 hunks)packages/plugins/resource/vite.config.ts(1 hunks)packages/register/src/constants.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-12-14T05:53:28.501Z
Learnt from: gene9831
PR: opentiny/tiny-engine#917
File: docs/开始/快速上手.md:31-31
Timestamp: 2024-12-14T05:53:28.501Z
Learning: The latest stable version of `opentiny/tiny-engine-cli` is `2.0.0`, and documentation should reference this version instead of any release candidates.
Applied to files:
packages/plugins/resource/package.json
📚 Learning: 2024-09-30T07:51:10.036Z
Learnt from: chilingling
PR: opentiny/tiny-engine#837
File: packages/vue-generator/src/plugins/genDependenciesPlugin.js:66-66
Timestamp: 2024-09-30T07:51:10.036Z
Learning: In the `tiny-engine` project, `opentiny/tiny-engine-dsl-vue` refers to the current package itself, and importing types from it may cause circular dependencies.
Applied to files:
packages/plugins/resource/package.jsonpackages/build/vite-config/src/vite-plugins/devAliasPlugin.js
🧬 Code graph analysis (4)
packages/layout/src/defaultLayout.js (1)
packages/register/src/constants.ts (1)
META_APP(25-82)
packages/plugins/resource/src/js/http.ts (3)
packages/configurator/src/source-select-configurator/http.ts (2)
fetchResourceListByGroupId(19-19)fetchResourceGroupByAppId(22-22)packages/register/src/common.ts (2)
getMetaApi(20-30)getMergeMeta(53-55)packages/register/src/constants.ts (1)
META_SERVICE(1-23)
packages/configurator/src/source-select-configurator/http.ts (3)
packages/plugins/resource/src/js/http.ts (2)
fetchResourceListByGroupId(19-20)fetchResourceGroupByAppId(54-57)packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-23)
packages/design-core/registry.js (1)
packages/plugins/materials/src/composable/types.ts (1)
Resource(49-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (20)
packages/plugins/resource/meta.js (2)
1-6: LGTM — concise, consistent metadata.
2-5: Verify plugin id uniqueness & icon availability
- ID occurrences (expected): packages/plugins/resource/meta.js (definition), packages/design-core/registry.js, packages/register/src/constants.ts — no other duplicate id found.
- Icon token 'plugin-icon-resource' referenced in packages/plugins/resource/meta.js and packages/plugins/resource/src/Main.vue; no SVG/asset or icon registration for that token was found — register the icon in the app's icon registry/sprite or add the SVG asset.
packages/plugins/resource/vite.config.ts (2)
27-29: Harden __dirname usage for ESM (package.json sets "type": "module")packages/plugins/resource/vite.config.ts — package.json contains "type": "module"; __dirname is undefined at runtime. Replace with:
-import path from 'path' +import path from 'path' +import { fileURLToPath } from 'node:url' +const __dirname = path.dirname(fileURLToPath(import.meta.url))⛔ Skipped due to learnings
Learnt from: hexqi PR: opentiny/tiny-engine#1501 File: mockServer/src/tool/Common.js:79-82 Timestamp: 2025-07-03T09:22:59.512Z Learning: In the tiny-engine project, the mockServer code uses ES6 import syntax but is compiled to CommonJS output. This means CommonJS globals like `__dirname` are available at runtime, while ES6 module-specific features like `import.meta` would cause runtime errors.
33-37: Ensure style.css is published with the packagepackages/plugins/resource/vite.config.ts imports "./style.css"; packages/plugins/resource/package.json has a "files" array — confirm that style.css (or the built dist folder containing it) is included in that array, otherwise consumers will see a missing-module error.
packages/plugins/resource/src/js/http.ts (1)
45-45: Confirm server expects GET for destructive operations.
If DELETE is supported, switch methods to avoid cache/proxy issues.-export const deleteResource = (id: number) => getMetaApi(META_SERVICE.Http).get(`${baseUrl}/resource/delete/${id}`) +export const deleteResource = (id: number) => getMetaApi(META_SERVICE.Http).delete(`${baseUrl}/resource/delete/${id}`) @@ -export const deleteResourceGroup = (id: number) => - getMetaApi(META_SERVICE.Http).get(`${baseUrl}/resource-group/delete/${id}`) +export const deleteResourceGroup = (id: number) => + getMetaApi(META_SERVICE.Http).delete(`${baseUrl}/resource-group/delete/${id}`)Also applies to: 76-77
packages/configurator/src/index.ts (1)
32-32: Good addition — SourceSelectConfigurator imported and re-exported.Also applies to: 69-69
packages/plugins/resource/index.ts (1)
13-19: LGTM — clean plugin entry wiring.packages/design-core/package.json (2)
60-60: Dependency added — looks correct.
60-60: Resolved — registry and dev alias are wired correctly.
Registry references Resource (packages/design-core/registry.js:43,165); re-export present (packages/design-core/re-export.js:30); dev alias maps '@opentiny/tiny-engine-plugin-resource' → packages/plugins/resource/index.ts (packages/build/vite-config/src/vite-plugins/devAliasPlugin.js:32).packages/register/src/constants.ts (1)
70-72: LGTM — META_APP.Resource mapping added.packages/layout/src/defaultLayout.js (1)
15-16: LGTM — Resource added to left/top plugin stack.packages/build/vite-config/src/vite-plugins/devAliasPlugin.js (1)
32-32: LGTM — build-time resolution confirmed
Alias target exists (packages/plugins/resource/index.ts); packages/design-core/package.json declares "@opentiny/tiny-engine-plugin-resource": "workspace:*" and re-exports it (packages/design-core/re-export.js:30), so build-time resolution will not rely on the dev-only alias (packages/build/vite-config/src/vite-plugins/devAliasPlugin.js:32).packages/design-core/re-export.js (1)
30-30: LGTM.
Re-export aligns with other plugins and the dev alias.packages/design-core/registry.js (2)
43-44: LGTM.
Importing Resource via re-export is consistent with existing pattern.
164-166: Gating key and meta id consistent; layout ordering confirmed.
Resource is mapped to 'engine.plugins.resource' in packages/register/src/constants.ts (line 71) and packages/plugins/resource/meta.js declares id: 'engine.plugins.resource' (line 2). defaultLayout lists META_APP.State then META_APP.Resource (adjacent), so ordering appears intentional.packages/canvas/render/src/builtin/builtin.json (1)
480-495: Scope selector to images, add i18n, and provide a fallback if Resource plugin is unavailable
- File: packages/canvas/render/src/builtin/builtin.json (≈lines 480–495) — add en_US label/description and restrict the SourceSelectConfigurator to images if supported; otherwise use InputConfigurator as a fallback.
Apply (only if SourceSelectConfigurator supports these props):
"label": { "text": { - "zh_CN": "资源选择" + "zh_CN": "资源选择", + "en_US": "Resource" } }, "cols": 12, "rules": [], "widget": { - "component": "SourceSelectConfigurator", - "props": {} + "component": "SourceSelectConfigurator", + "props": { + "accept": "image/*", + "resourceTypes": ["image"] + } }, "description": { - "zh_CN": "src路径" + "zh_CN": "src路径", + "en_US": "Image src URL" }, "labelPosition": "top"Verify configurator export and fallback availability (run and attach results):
rg -n "SourceSelectConfigurator" packages/configurator -S || true rg -n "export .*SourceSelectConfigurator" packages/configurator -S || true rg -n '"component"\s*:\s*"InputConfigurator"' packages/canvas/render/src/builtin/builtin.json -n || truedesigner-demo/public/mock/bundle.json (3)
4442-4444: Good move: switch to SourceSelectConfigurator.This aligns the Img.src with the new resource workflow and improves UX for picking managed assets.
4442-4444: Incorrect: SourceSelectConfigurator doesn't accept resourceType/returnFormatComponent only defines props:
modelValue(type String, default '') — packages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue (props around lines ~108–111) and is exported. Either add the requested props to the component or pass the selected URL viamodelValueinstead.Likely an incorrect or invalid review comment.
4447-4449: Keep labelPosition: "top" for this SourceSelectConfigurator.Verified other SourceSelectConfigurator usage (packages/canvas/render/src/builtin/builtin.json) also uses "labelPosition": "top" — no change needed.
packages/plugins/resource/src/ResourceList.vue (1)
321-349: Confirmed: updateResourceGroup supports updating resource list (including deletion); current retained-list payload is valid.updateResourceGroup is a PUT to /material-center/api/resource-group/update/{id} and its comment states it "modifies resource group info — including deletion". There is also deleteResource (GET /material-center/api/resource/delete/{id}) for single deletes and no batch-delete endpoint found — using one PUT with the remaining resource ids is reasonable.
packages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue
Show resolved
Hide resolved
packages/configurator/src/source-select-configurator/SourceSelectConfigurator.vue
Show resolved
Hide resolved
5f3f840 to
aa7bd7e
Compare
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、需要用户添加并管理静态资源并在项目中使用;
2、需要用户将静态资源应用到组件中;
What is the current behavior?
Issue Number: N/A
What is the new behavior?
插件栏有了添加静态资源的能力,目前只支持图片格式的静态资源,支持静态资源分组、增加、删除
物料的图片组件的src支持选择静态资源并绑定
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Enhancements
Chores