Conversation
imsyy
commented
Dec 4, 2025
Summary of ChangesHello @imsyy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在全面提升应用程序的下载管理体验。通过引入一个全新的下载管理页面和统一的下载弹窗,用户可以更直观地查看和控制下载任务。核心下载逻辑被封装在 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
这个 PR 对下载功能进行了重大的重构和改进,值得称赞。通过引入 DownloadManager 单例和新的 DownloadModal 组件,代码变得更加模块化和可维护。新的 downloading 页面能够清晰地展示所有下载任务的状态,并提供了重试和取消等操作,极大地提升了用户体验。
我在代码中发现了一些可以改进的地方:
- 新的下载弹窗不再显示不同音质的文件大小,这是一个小的功能退步。
data.ts中,标记下载失败的状态没有被持久化,可能导致应用重启后状态丢失。DownloadManager中,使用播放链接下载时缺少了对音质是否支持的校验。
总的来说,这是一次非常出色的重构,解决了之前批量下载的许多限制。修复上述几个小问题后,下载管理功能将更加完善和健壮。
| const qualityOptions = computed(() => { | ||
| const levels = pick(songLevelData, ["l", "m", "h", "sq", "hr", "je", "sk", "db", "jm"]); | ||
| return getSongLevelsData(levels).map((item) => ({ | ||
| label: item.name, | ||
| value: item.value, | ||
| size: undefined, | ||
| })); | ||
| }); |
| const item = this.downloadingSongs.find((item) => item.song.id === songId); | ||
| if (item) { | ||
| item.status = "failed"; | ||
| item.progress = 0; | ||
| item.transferred = "0MB"; | ||
| item.totalSize = "0MB"; | ||
| } | ||
| }, | ||
| // 重置下载任务状态(用于重试) |
There was a problem hiding this comment.
问题:下载失败状态未持久化
markDownloadFailed 方法在将歌曲状态标记为 failed 后,没有将更新后的 downloadingSongs 数组持久化到 musicDB。这可能导致在应用重启后,失败的任务状态会丢失,用户无法看到失败的记录或进行重试。
建议在 markDownloadFailed 方法中也调用 musicDB.setItem 来保存状态。
markDownloadFailed(songId: number) {
const item = this.downloadingSongs.find((item) => item.song.id === songId);
if (item) {
item.status = "failed";
item.progress = 0;
item.transferred = "0MB";
item.totalSize = "0MB";
musicDB.setItem("downloadingSongs", cloneDeep(this.downloadingSongs));
}
},| if (usePlayback) { | ||
| try { | ||
| const result = await songUrl(song.id, levelName as any); | ||
| if (result.code === 200 && result?.data?.[0]?.url) { | ||
| url = result.data[0].url; | ||
| type = (result.data[0].type || result.data[0].encodeType || "mp3").toLowerCase(); | ||
| } | ||
| } catch (e) { | ||
| console.error("Error fetching playback url for download:", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
问题:使用播放链接下载时缺少音质支持校验
在使用播放链接进行下载时 (usePlayback 为 true),代码直接尝试使用 songUrl API,但没有校验当前选择的音质是否被该 API 支持。旧的实现 (download.ts) 中存在一个 allowedLevels 数组用于校验。
缺少这个校验可能会导致在选择某些特定音质(如 dolby)时,songUrl API 调用失败,而代码会继续尝试使用标准下载接口,这虽然是预期的回退行为,但增加了一次不必要的失败请求。
建议恢复对音质支持的校验逻辑,以避免不必要的 API 调用。
const allowedPlaybackLevels = [
"standard",
"higher",
"exhigh",
"lossless",
"hires",
"jyeffect",
"sky",
"jymaster",
];
if (usePlayback && allowedPlaybackLevels.includes(levelName)) {
try {
const result = await songUrl(song.id, levelName as any);
if (result.code === 200 && result?.data?.[0]?.url) {
url = result.data[0].url;
type = (result.data[0].type || result.data[0].encodeType || "mp3").toLowerCase();
}
} catch (e) {
console.error("Error fetching playback url for download:", e);
}
}