Skip to content
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

mod: move episode comments to menu #572

Merged
merged 3 commits into from
Jan 8, 2025
Merged

Conversation

ErBWs
Copy link
Contributor

@ErBWs ErBWs commented Jan 8, 2025

修复上一个 PR 的 Timer 问题

移动评论区,和选集放在一起。这样做的好处是比较整齐,后续有 bangumi 登录之后也可以增加发送评论的功能,会比较好看

  • maintain state 和 keep alive 是为了避免反复构建评论区造成的卡顿
  • 手机端全屏只会呼出选集,不会呼出评论区
image image image

@Predidit
Copy link
Owner

Predidit commented Jan 8, 2025

这个PR非常漂亮

但是按照我对 tab 行为的理解

这样会导致每次观看番剧时自动加载评论

我们能否修改为只在切换到评论选项卡时进行加载?

@ErBWs
Copy link
Contributor Author

ErBWs commented Jan 8, 2025

不会,它只会在切换过去的时候进行加载,然后 keep alive 直到 video_page 被 dispose

@Predidit Predidit merged commit 224e66c into Predidit:main Jan 8, 2025
6 checks passed
@Predidit
Copy link
Owner

Predidit commented Jan 8, 2025

@ErBWs

切换分集后评论区加载会出问题

@ErBWs
Copy link
Contributor Author

ErBWs commented Jan 8, 2025

我看看怎么解决,我测试总是忘记切分集()

@ErBWs ErBWs deleted the mod-player branch January 8, 2025 07:36
@Predidit
Copy link
Owner

Predidit commented Jan 8, 2025

@ErBWs

解耦和重构应该完成了

现在和 media_kit 直接耦合的部分已经限制在了 player_controller.dart 和 player_item_surface.dart 中, 外部目前也不再存在对 mediaPlayer 对象的直接引用

@ErBWs
Copy link
Contributor Author

ErBWs commented Jan 8, 2025

太好了。

另外切集这个问题有点麻烦,目前还没找到有效的解决办法,keep alive 造成的问题有点多

@ErBWs
Copy link
Contributor Author

ErBWs commented Jan 8, 2025

其实 keep alive 主要就是为了解决 tab 切换时的卡顿问题,虽说这个卡顿在 Release 模式下会缓解但我不确定在性能较差的设备上会不会出现卡顿。为什么 bottom sheet 就不会卡顿呢,不理解

@Predidit
Copy link
Owner

Predidit commented Jan 8, 2025

Tab 就是很容易卡,在 flutter framework 层面的实现里也看不出什么。在组件树中维护所有 tabBody 可能是卡顿的原因?

不然可以考虑做一个伪 tab 实现,不再维护所有 tabBody ,也不考虑 tab 过渡动画的实现问题

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants