feat(webui): add compact view for installed plugins - #9714
feat(webui): add compact view for installed plugins#9714Alchuang22-dev wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
pluginCardDensityvalues ('detailed'/'compact') are hardcoded in multiple places; consider centralizing them in a shared constant or type to avoid future drift or typos. - The activation switch logic in
ExtensionCard.vueis now duplicated between detailed and compact modes; extracting this into a single reusable subcomponent or template block would simplify maintenance and keep behavior aligned. - In
ExtensionPage.vueandMarketPluginsTab.vue, the legacy table-view bindings are only commented out; once you’re confident they’re obsolete, fully removing them (and any now-unused reactive state) would reduce noise and potential confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `pluginCardDensity` values (`'detailed'` / `'compact'`) are hardcoded in multiple places; consider centralizing them in a shared constant or type to avoid future drift or typos.
- The activation switch logic in `ExtensionCard.vue` is now duplicated between detailed and compact modes; extracting this into a single reusable subcomponent or template block would simplify maintenance and keep behavior aligned.
- In `ExtensionPage.vue` and `MarketPluginsTab.vue`, the legacy table-view bindings are only commented out; once you’re confident they’re obsolete, fully removing them (and any now-unused reactive state) would reduce noise and potential confusion.
## Individual Comments
### Comment 1
<location path="dashboard/tests/extensionPreferenceStorage.test.mjs" line_range="69" />
<code_context>
+ assert.equal(readPluginCardDensity(storage), 'compact');
+});
+
+test('readPluginCardDensity falls back for invalid or unavailable storage', () => {
+ assert.equal(
+ readPluginCardDensity({
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test case for `readPluginCardDensity` when `getItem` returns `null` to explicitly cover that edge case.
The current test already covers invalid string values, `null` storage, and exceptions from `getItem`. Please also add an assertion for the case where `getItem` itself returns `null`, to explicitly document the fallback-to-`'detailed'` behavior and prevent future regressions.
Suggested implementation:
```javascript
test('readPluginCardDensity falls back for invalid or unavailable storage', () => {
assert.equal(
readPluginCardDensity({
getItem() {
return 'unsupported';
},
}),
'detailed',
);
assert.equal(
readPluginCardDensity({
getItem() {
return null;
},
}),
'detailed',
);
assert.equal(readPluginCardDensity(null), 'detailed');
assert.equal(
readPluginCardDensity({
getItem() {
```
The replacement above assumes the rest of the test (after the final `getItem() {` in the snippet) remains unchanged and continues to cover the exception-from-`getItem` case. Please ensure that the trailing object and assertion are still valid after this insertion (e.g., the `getItem` implementation that throws, and the closing parentheses/braces for the assertion and test function).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| assert.equal(readPluginCardDensity(storage), 'compact'); | ||
| }); | ||
|
|
||
| test('readPluginCardDensity falls back for invalid or unavailable storage', () => { |
There was a problem hiding this comment.
suggestion (testing): Add a test case for readPluginCardDensity when getItem returns null to explicitly cover that edge case.
The current test already covers invalid string values, null storage, and exceptions from getItem. Please also add an assertion for the case where getItem itself returns null, to explicitly document the fallback-to-'detailed' behavior and prevent future regressions.
Suggested implementation:
test('readPluginCardDensity falls back for invalid or unavailable storage', () => {
assert.equal(
readPluginCardDensity({
getItem() {
return 'unsupported';
},
}),
'detailed',
);
assert.equal(
readPluginCardDensity({
getItem() {
return null;
},
}),
'detailed',
);
assert.equal(readPluginCardDensity(null), 'detailed');
assert.equal(
readPluginCardDensity({
getItem() {The replacement above assumes the rest of the test (after the final getItem() { in the snippet) remains unchanged and continues to cover the exception-from-getItem case. Please ensure that the trailing object and assertion are still valid after this insertion (e.g., the getItem implementation that throws, and the closing parentheses/braces for the assertion and test function).
Fix #9708 issue
Modifications / 改动点
将旧版本的 list 视图改为和重构后插件 webui 配合的紧凑视图,这样一页就能够显示更多的插件了。
同时清理了 Vue 中的已失效 element。
Screenshots or Test Results / 运行截图或测试结果
当然,如果按用户在 issue 中的描述,应该改成以下视图:
但是在栏数增加到4时,很容易出现覆盖问题。因此 list 视图的效果更好。
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Introduce a switchable detailed/compact layout for installed plugin cards with persisted density preference and updated UI grid.
New Features:
Enhancements:
Tests: