feat(player): 添加播放时间格式切换功能 #680
Conversation
Summary of ChangesHello @MoYingJi, 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
本次 PR 添加了播放器时间格式的切换功能，整体实现清晰。通过在 settingStore 中添加新状态，并在 PlaySetting 页面提供配置项，用户可以自定义主播放器和全屏播放器的时间显示格式。点击时间区域可以快速循环切换格式，提升了用户体验。代码结构方面，将时间格式化逻辑集中在 src/utils/format.ts 中是很好的实践。我发现了一些可以改进的地方，主要涉及代码健壮性、性能优化和代码复用，具体请看我的评论。这些修改将有助于提升应用的稳定性和可维护性。
| const toggleTimeFormat = () => { | ||
| const currentIndex = TIME_FORMATS.indexOf(settingStore.timeFormatMainPlayer) | ||
| settingStore.timeFormatMainPlayer = TIME_FORMATS[(currentIndex + 1) % TIME_FORMATS.length]; | ||
| }; |
There was a problem hiding this comment.
toggleTimeFormat 函数中的逻辑与 PlayerControl.vue 中的几乎完全相同，只是操作的 settingStore 属性不同。为了遵循 DRY (Don't Repeat Yourself) 原则并提高代码的可维护性，建议将这个切换逻辑提取到一个公共的工具函数中，例如在 src/utils/format.ts 中创建一个 getNextTimeFormat 函数。这样两个组件都可以复用这个逻辑。
| const toggleTimeFormat = () => { | ||
| const currentIndex = TIME_FORMATS.indexOf(settingStore.timeFormatFullPlayer); | ||
| settingStore.timeFormatFullPlayer = TIME_FORMATS[(currentIndex + 1) % TIME_FORMATS.length]; | ||
| }; |
There was a problem hiding this comment.
toggleTimeFormat 函数中的逻辑与 MainPlayer.vue 中的几乎完全相同。为了提高代码的复用性和可维护性，建议将此逻辑抽象成一个可复用的工具函数，并放置在 src/utils/format.ts 文件中。
| export const displayTimeFormat = (format: TimeFormat): [TimeDisplayType, TimeDisplayType] => { | ||
| switch (format) { | ||
| case "current-total": return ["current", "total"]; | ||
| case "remaining-total": return ["remaining", "total"]; | ||
| case "current-remaining": return ["current", "remaining"]; | ||
| } |
There was a problem hiding this comment.
这个 switch 语句没有处理所有 TimeFormat 类型之外的意外值。虽然 TypeScript 的类型系统可以在编译时提供保障，但考虑到设置可能从 localStorage 中读取，数据可能存在损坏或来自旧版本的风险。为了增强代码的健壮性，建议添加一个 default 分支作为回退，以防止在 format 值为意外值时函数返回 undefined 导致运行时错误。
| export const displayTimeFormat = (format: TimeFormat): [TimeDisplayType, TimeDisplayType] => { | |
| switch (format) { | |
| case "current-total": return ["current", "total"]; | |
| case "remaining-total": return ["remaining", "total"]; | |
| case "current-remaining": return ["current", "remaining"]; | |
| } | |
| export const displayTimeFormat = (format: TimeFormat): [TimeDisplayType, TimeDisplayType] => { | |
| switch (format) { | |
| case "current-total": return ["current", "total"]; | |
| case "remaining-total": return ["remaining", "total"]; | |
| case "current-remaining": return ["current", "remaining"]; | |
| default: return ["current", "total"]; | |
| } | |
| }; |
| export const getTimeDisplay = ( | ||
| format: () => TimeFormat, statusStore: { currentTime: number, duration: number } | ||
| ) => (index: number) => computed(() => { | ||
| const display = displayTimeFormat(format())[index]; | ||
| switch (display) { | ||
| case "current": return msToTime(statusStore.currentTime); | ||
| case "total": return msToTime(statusStore.duration); | ||
| case "remaining": return "-" + msToTime(statusStore.duration - statusStore.currentTime); | ||
| } | ||
| }); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1 participant
No description provided.