-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
perf: list view mode toggle with localStorage support in ExtensionPage #4288
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
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.
Hey - 我发现了 3 个问题,并给出了一些整体反馈:
itemsPerPage已经从脚本中移除,但很可能仍然在分页逻辑/模板中被使用;如果确实不再需要,请一并移除依赖它的逻辑,否则请重新引入这个 ref,以避免运行时使用到undefined。- 你在多个地方使用相同的 key 字符串访问
localStorage;建议将该 key 以及 get/set 逻辑抽取到一个小的辅助函数中,以避免重复,并方便后续行为变更。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- `itemsPerPage` is removed from the script but likely still used in pagination logic/template; if it's no longer needed, remove the dependent logic, otherwise reintroduce the ref to avoid runtime `undefined` usage.
- You access `localStorage` in multiple places with the same key string; consider extracting the key and the get/set logic into a small helper to avoid duplication and make it easier to change behavior later.
## Individual Comments
### Comment 1
<location> `dashboard/src/views/ExtensionPage.vue:80-90` </location>
<code_context>
// 新增变量支持列表视图
-const isListView = ref(false);
+// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
+const getInitialListViewMode = () => {
+ if (typeof window !== 'undefined' && window.localStorage) {
+ return localStorage.getItem('pluginListViewMode') === 'true';
+ }
</code_context>
<issue_to_address>
**suggestion:** 考虑增强对 localStorage 访问的健壮性,以避免在受限环境下出现运行时错误。
`localStorage.getItem` 在某些环境中仍可能抛出异常(例如 Safari 隐私模式或存储被阻止时)。请将此调用包裹在 try/catch 中,并在访问失败时返回 `false`,以避免初始化期间出现运行时错误。
```suggestion
// 新增变量支持列表视图
// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
const getInitialListViewMode = () => {
if (typeof window === "undefined" || !window.localStorage) {
return false;
}
try {
return window.localStorage.getItem("pluginListViewMode") === "true";
} catch (_error) {
// 在受限环境(例如隐私模式或被阻止的存储)中安全回退
return false;
}
};
const isListView = ref(getInitialListViewMode());
const pluginSearch = ref("");
const loading_ = ref(false);
```
</issue_to_address>
### Comment 2
<location> `dashboard/src/views/ExtensionPage.vue:904-906` </location>
<code_context>
});
+// 监听显示模式变化并保存到 localStorage
+watch(isListView, (newVal) => {
+ if (typeof window !== 'undefined' && window.localStorage) {
+ localStorage.setItem('pluginListViewMode', String(newVal));
+ }
+});
</code_context>
<issue_to_address>
**suggestion:** 对视图模式的持久化逻辑进行去重并增强健壮性。
这里复用了 `getInitialListViewMode` 中的 localStorage key 和访问模式。建议将 key 以及加载/保存逻辑集中到一个共享的辅助函数中,这样变更只需在一个地方维护;同时像 getter 一样,用 try/catch 包裹 `setItem`,以便在存储不可用时避免运行时抛错。
建议实现:
```
// 监听显示模式变化并保存到 localStorage
watch(isListView, (newVal) => {
persistListViewMode(newVal);
});
```
```
// 新增变量支持列表视图
// localStorage 键名常量,避免重复字符串
const PLUGIN_LIST_VIEW_MODE_KEY = 'pluginListViewMode';
// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
const getInitialListViewMode = () => {
if (typeof window === 'undefined' || !window.localStorage) {
return false;
}
try {
return localStorage.getItem(PLUGIN_LIST_VIEW_MODE_KEY) === 'true';
} catch {
// 读取异常时回退到默认视图
return false;
}
};
// 持久化显示模式到 localStorage(安全封装)
const persistListViewMode = (value) => {
if (typeof window === 'undefined' || !window.localStorage) {
return;
}
try {
localStorage.setItem(PLUGIN_LIST_VIEW_MODE_KEY, String(value));
} catch {
// 写入异常时静默失败,避免影响运行时
}
};
```
如果该文件(或其他地方)还有任何直接使用字面量 `'pluginListViewMode'` 或手动为此视图模式调用 `localStorage.getItem`/`setItem` 的代码,也应该切换为使用 `PLUGIN_LIST_VIEW_MODE_KEY`、`getInitialListViewMode` 或 `persistListViewMode`,以保证逻辑完全集中且一致。
</issue_to_address>
### Comment 3
<location> `dashboard/src/views/ExtensionPage.vue:1029-1033` </location>
<code_context>
<template v-slot:item.name="{ item }">
<div class="d-flex align-center py-2">
+ <div v-if="item.logo" class="mr-3" style="flex-shrink: 0;">
+ <img :src="item.logo" :alt="item.name"
+ style="height: 40px; width: 40px; border-radius: 8px; object-fit: cover;" />
+ </div>
+ <div v-else class="mr-3" style="flex-shrink: 0;">
+ <img :src="defaultPluginIcon" :alt="item.name"
+ style="height: 40px; width: 40px; border-radius: 8px; object-fit: cover;" />
</code_context>
<issue_to_address>
**suggestion:** 通过条件设置 src 来避免重复的 logo 容器标记。
由于只有 `img` 的 src 不同,你可以保留一个共同的外层容器,并通过形如 `:src="item.logo || defaultPluginIcon"` 的方式条件设置 src。这样可以避免重复的标记,并将后续对布局/样式的修改集中在一处。
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进之后的评审。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
itemsPerPageis removed from the script but likely still used in pagination logic/template; if it's no longer needed, remove the dependent logic, otherwise reintroduce the ref to avoid runtimeundefinedusage.- You access
localStoragein multiple places with the same key string; consider extracting the key and the get/set logic into a small helper to avoid duplication and make it easier to change behavior later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- `itemsPerPage` is removed from the script but likely still used in pagination logic/template; if it's no longer needed, remove the dependent logic, otherwise reintroduce the ref to avoid runtime `undefined` usage.
- You access `localStorage` in multiple places with the same key string; consider extracting the key and the get/set logic into a small helper to avoid duplication and make it easier to change behavior later.
## Individual Comments
### Comment 1
<location> `dashboard/src/views/ExtensionPage.vue:80-90` </location>
<code_context>
// 新增变量支持列表视图
-const isListView = ref(false);
+// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
+const getInitialListViewMode = () => {
+ if (typeof window !== 'undefined' && window.localStorage) {
+ return localStorage.getItem('pluginListViewMode') === 'true';
+ }
</code_context>
<issue_to_address>
**suggestion:** Consider hardening localStorage access to avoid runtime errors in restricted environments.
`localStorage.getItem` can still throw in some environments (e.g., Safari private mode or when storage is blocked). Please wrap this call in a try/catch and return `false` if access fails to avoid runtime errors during initialization.
```suggestion
// 新增变量支持列表视图
// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
const getInitialListViewMode = () => {
if (typeof window === "undefined" || !window.localStorage) {
return false;
}
try {
return window.localStorage.getItem("pluginListViewMode") === "true";
} catch (_error) {
// 在受限环境(例如隐私模式或被阻止的存储)中安全回退
return false;
}
};
const isListView = ref(getInitialListViewMode());
const pluginSearch = ref("");
const loading_ = ref(false);
```
</issue_to_address>
### Comment 2
<location> `dashboard/src/views/ExtensionPage.vue:904-906` </location>
<code_context>
});
+// 监听显示模式变化并保存到 localStorage
+watch(isListView, (newVal) => {
+ if (typeof window !== 'undefined' && window.localStorage) {
+ localStorage.setItem('pluginListViewMode', String(newVal));
+ }
+});
</code_context>
<issue_to_address>
**suggestion:** Deduplicate and harden the persistence logic for the view mode.
This duplicates the `getInitialListViewMode` localStorage key and access pattern. Consider centralizing the key and load/save logic in a shared helper so changes stay in one place, and wrap `setItem` in try/catch (as with the getter) to handle cases where storage is unavailable without throwing at runtime.
Suggested implementation:
```
// 监听显示模式变化并保存到 localStorage
watch(isListView, (newVal) => {
persistListViewMode(newVal);
});
```
```
// 新增变量支持列表视图
// localStorage 键名常量,避免重复字符串
const PLUGIN_LIST_VIEW_MODE_KEY = 'pluginListViewMode';
// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
const getInitialListViewMode = () => {
if (typeof window === 'undefined' || !window.localStorage) {
return false;
}
try {
return localStorage.getItem(PLUGIN_LIST_VIEW_MODE_KEY) === 'true';
} catch {
// 读取异常时回退到默认视图
return false;
}
};
// 持久化显示模式到 localStorage(安全封装)
const persistListViewMode = (value) => {
if (typeof window === 'undefined' || !window.localStorage) {
return;
}
try {
localStorage.setItem(PLUGIN_LIST_VIEW_MODE_KEY, String(value));
} catch {
// 写入异常时静默失败,避免影响运行时
}
};
```
If there are any other places in this file (or elsewhere) directly using the literal `'pluginListViewMode'` or manually calling `localStorage.getItem`/`setItem` for this view mode, they should be switched to use `PLUGIN_LIST_VIEW_MODE_KEY`, `getInitialListViewMode`, or `persistListViewMode` to keep the logic fully centralized and consistent.
</issue_to_address>
### Comment 3
<location> `dashboard/src/views/ExtensionPage.vue:1029-1033` </location>
<code_context>
<template v-slot:item.name="{ item }">
<div class="d-flex align-center py-2">
+ <div v-if="item.logo" class="mr-3" style="flex-shrink: 0;">
+ <img :src="item.logo" :alt="item.name"
+ style="height: 40px; width: 40px; border-radius: 8px; object-fit: cover;" />
+ </div>
+ <div v-else class="mr-3" style="flex-shrink: 0;">
+ <img :src="defaultPluginIcon" :alt="item.name"
+ style="height: 40px; width: 40px; border-radius: 8px; object-fit: cover;" />
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating the logo wrapper markup by using a conditional src instead.
Since only the `img` src differs, you can keep a single wrapper and set the src conditionally, e.g. `:src="item.logo || defaultPluginIcon"`. This avoids duplicated markup and keeps future layout/style changes in one place.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // 新增变量支持列表视图 | ||
| const isListView = ref(false); | ||
| // 从 localStorage 恢复显示模式,默认为 false(卡片视图) | ||
| const getInitialListViewMode = () => { | ||
| if (typeof window !== 'undefined' && window.localStorage) { | ||
| return localStorage.getItem('pluginListViewMode') === 'true'; | ||
| } | ||
| return false; | ||
| }; | ||
| const isListView = ref(getInitialListViewMode()); | ||
| const pluginSearch = ref(""); | ||
| const loading_ = ref(false); |
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.
suggestion: 考虑增强对 localStorage 访问的健壮性,以避免在受限环境下出现运行时错误。
localStorage.getItem 在某些环境中仍可能抛出异常(例如 Safari 隐私模式或存储被阻止时)。请将此调用包裹在 try/catch 中,并在访问失败时返回 false,以避免初始化期间出现运行时错误。
| // 新增变量支持列表视图 | |
| const isListView = ref(false); | |
| // 从 localStorage 恢复显示模式,默认为 false(卡片视图) | |
| const getInitialListViewMode = () => { | |
| if (typeof window !== 'undefined' && window.localStorage) { | |
| return localStorage.getItem('pluginListViewMode') === 'true'; | |
| } | |
| return false; | |
| }; | |
| const isListView = ref(getInitialListViewMode()); | |
| const pluginSearch = ref(""); | |
| const loading_ = ref(false); | |
| // 新增变量支持列表视图 | |
| // 从 localStorage 恢复显示模式,默认为 false(卡片视图) | |
| const getInitialListViewMode = () => { | |
| if (typeof window === "undefined" || !window.localStorage) { | |
| return false; | |
| } | |
| try { | |
| return window.localStorage.getItem("pluginListViewMode") === "true"; | |
| } catch (_error) { | |
| // 在受限环境(例如隐私模式或被阻止的存储)中安全回退 | |
| return false; | |
| } | |
| }; | |
| const isListView = ref(getInitialListViewMode()); | |
| const pluginSearch = ref(""); | |
| const loading_ = ref(false); |
Original comment in English
suggestion: Consider hardening localStorage access to avoid runtime errors in restricted environments.
localStorage.getItem can still throw in some environments (e.g., Safari private mode or when storage is blocked). Please wrap this call in a try/catch and return false if access fails to avoid runtime errors during initialization.
| // 新增变量支持列表视图 | |
| const isListView = ref(false); | |
| // 从 localStorage 恢复显示模式,默认为 false(卡片视图) | |
| const getInitialListViewMode = () => { | |
| if (typeof window !== 'undefined' && window.localStorage) { | |
| return localStorage.getItem('pluginListViewMode') === 'true'; | |
| } | |
| return false; | |
| }; | |
| const isListView = ref(getInitialListViewMode()); | |
| const pluginSearch = ref(""); | |
| const loading_ = ref(false); | |
| // 新增变量支持列表视图 | |
| // 从 localStorage 恢复显示模式,默认为 false(卡片视图) | |
| const getInitialListViewMode = () => { | |
| if (typeof window === "undefined" || !window.localStorage) { | |
| return false; | |
| } | |
| try { | |
| return window.localStorage.getItem("pluginListViewMode") === "true"; | |
| } catch (_error) { | |
| // 在受限环境(例如隐私模式或被阻止的存储)中安全回退 | |
| return false; | |
| } | |
| }; | |
| const isListView = ref(getInitialListViewMode()); | |
| const pluginSearch = ref(""); | |
| const loading_ = ref(false); |
| watch(isListView, (newVal) => { | ||
| if (typeof window !== 'undefined' && window.localStorage) { | ||
| localStorage.setItem('pluginListViewMode', String(newVal)); |
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.
suggestion: 对视图模式的持久化逻辑进行去重并增强健壮性。
这里复用了 getInitialListViewMode 中的 localStorage key 和访问模式。建议将 key 以及加载/保存逻辑集中到一个共享的辅助函数中,这样变更只需在一个地方维护;同时像 getter 一样,用 try/catch 包裹 setItem,以便在存储不可用时避免运行时抛错。
建议实现:
// 监听显示模式变化并保存到 localStorage
watch(isListView, (newVal) => {
persistListViewMode(newVal);
});
// 新增变量支持列表视图
// localStorage 键名常量,避免重复字符串
const PLUGIN_LIST_VIEW_MODE_KEY = 'pluginListViewMode';
// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
const getInitialListViewMode = () => {
if (typeof window === 'undefined' || !window.localStorage) {
return false;
}
try {
return localStorage.getItem(PLUGIN_LIST_VIEW_MODE_KEY) === 'true';
} catch {
// 读取异常时回退到默认视图
return false;
}
};
// 持久化显示模式到 localStorage(安全封装)
const persistListViewMode = (value) => {
if (typeof window === 'undefined' || !window.localStorage) {
return;
}
try {
localStorage.setItem(PLUGIN_LIST_VIEW_MODE_KEY, String(value));
} catch {
// 写入异常时静默失败,避免影响运行时
}
};
如果该文件(或其他地方)还有任何直接使用字面量 'pluginListViewMode' 或手动为此视图模式调用 localStorage.getItem/setItem 的代码,也应该切换为使用 PLUGIN_LIST_VIEW_MODE_KEY、getInitialListViewMode 或 persistListViewMode,以保证逻辑完全集中且一致。
Original comment in English
suggestion: Deduplicate and harden the persistence logic for the view mode.
This duplicates the getInitialListViewMode localStorage key and access pattern. Consider centralizing the key and load/save logic in a shared helper so changes stay in one place, and wrap setItem in try/catch (as with the getter) to handle cases where storage is unavailable without throwing at runtime.
Suggested implementation:
// 监听显示模式变化并保存到 localStorage
watch(isListView, (newVal) => {
persistListViewMode(newVal);
});
// 新增变量支持列表视图
// localStorage 键名常量,避免重复字符串
const PLUGIN_LIST_VIEW_MODE_KEY = 'pluginListViewMode';
// 从 localStorage 恢复显示模式,默认为 false(卡片视图)
const getInitialListViewMode = () => {
if (typeof window === 'undefined' || !window.localStorage) {
return false;
}
try {
return localStorage.getItem(PLUGIN_LIST_VIEW_MODE_KEY) === 'true';
} catch {
// 读取异常时回退到默认视图
return false;
}
};
// 持久化显示模式到 localStorage(安全封装)
const persistListViewMode = (value) => {
if (typeof window === 'undefined' || !window.localStorage) {
return;
}
try {
localStorage.setItem(PLUGIN_LIST_VIEW_MODE_KEY, String(value));
} catch {
// 写入异常时静默失败,避免影响运行时
}
};
If there are any other places in this file (or elsewhere) directly using the literal 'pluginListViewMode' or manually calling localStorage.getItem/setItem for this view mode, they should be switched to use PLUGIN_LIST_VIEW_MODE_KEY, getInitialListViewMode, or persistListViewMode to keep the logic fully centralized and consistent.
closes: #4253
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.由 Sourcery 提供的摘要
在扩展页面中持久化扩展列表视图模式,并优化扩展表格的展示效果。
新功能:
localStorage记住插件列表视图模式,使其在会话之间保持一致。改进:
Original summary in English
Summary by Sourcery
Persist the extension list view mode and refine the extensions table presentation in the Extension page.
New Features:
Enhancements: