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

feat(ui): adaptive layout for video page #726

Merged
merged 4 commits into from
Feb 15, 2025

Conversation

ErBWs
Copy link
Contributor

@ErBWs ErBWs commented Feb 14, 2025

测试地狱

  • 只要窗口宽度大于高度就用侧边菜单,不考虑 breakpoint
  • gridDelegate 用 mainAxisExtent 解决电脑上调节窗口大小时高度不断变化导致定位集数位置错误
  • 用 Flexible 解决手机端横屏点击弹幕输入框呼出的键盘会挡住输入框的问题

其他的基本都写在注释里了

@Predidit
Copy link
Owner

Predidit commented Feb 15, 2025

看上去非常棒,我没有任何的意见。

我们先合并它,当特定设备出现问题时再 workaround 。

@Predidit Predidit merged commit 262f878 into Predidit:main Feb 15, 2025
6 checks passed
@ErBWs ErBWs deleted the feat-adaptive-video-page branch February 15, 2025 03:30
@Predidit
Copy link
Owner

这个PR似乎再次损坏了查看评论时发生剧集调整时的评论加载

@Predidit
Copy link
Owner

抱歉,应该不是这个PR的问题

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

打开评论区的情况下切换选集的确会出问题,但是切换一下详情和评论应该就能正常显示了

应该是加载放在了 initState 里的原因,打开评论区的情况下切集不会触发 initState

@Predidit
Copy link
Owner

我稍后会提交一个修复

@Predidit
Copy link
Owner

Predidit commented Feb 15, 2025

似乎没有我想的那么简单,特别是要保持懒加载不被破坏

将加载位置移动到 changeEpisode 就像弹幕那样解决一切问题,但是会破坏懒加载

这部分代码的所有者是你,你怎么看

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

或许可以加一个 RefreshIndicator,当 infoController.episodeCommentsList.isEmpty 的时候提示进行下拉刷新

@Predidit
Copy link
Owner

RefreshIndicator 完全不适合桌面设备

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

这样可以支持桌面设备

scrollBehavior: const ScrollBehavior().copyWith(scrollbars: false),

- scrollBehavior: const ScrollBehavior().copyWith(scrollbars: false), 
+ scrollBehavior: const ScrollBehavior().copyWith(
+   scrollbars: false,
+   dragDevices: {
+     PointerDeviceKind.mouse,
+     PointerDeviceKind.touch,
+   },
+ ),

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

Screen-2025-02-15-191955.mp4

@Predidit
Copy link
Owner

我在主分支上尝试了另一种修复

在 tab 的位置在评论面板时,切换分集将加载评论

不过这样会导致另一个问题,在查看评论后全屏。发生分集切换时评论将被自动加载,即使此时评论在全屏状态下被隐藏

这个问题有很多方法修复,但没有太优雅的实现

我在考虑要不要保持这一行为

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

也许可以与 showTabBody 一起做判断

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

// Check fullscreen when enter video page
// in case user use system controls to enter fullscreen outside video page
tabController = TabController(length: 2, vsync: this);
videoPageController.isDesktopFullscreen();

这两句可以上下换一下,和注释不符

// Query comments when tab is visible
// We need lazy load comments to avoid unnecessary API requests
// so we only query comments when the tab is visible rather than video is loaded
if (tabController.index == 1) {
infoController.queryBangumiEpisodeCommentsByID(infoController.bangumiItem.id, episode);
}

- if (tabController.index == 1) {
+ if (tabController.index == 1 && videoPageController.showTabBody) {

这样或许能解决全屏问题,并且相对优雅?

@Predidit
Copy link
Owner

这样似乎不解决上面提到的移动设备的问题

移动设备默认情况下竖屏 showTabBody 为 false 但是显示评论区

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

感觉 RefreshIndicator 是个好主意

tabController 应该解决了我一直想解决的关闭 menu 会重置 tab 选择的问题

@Predidit
Copy link
Owner

我想尽量避免使用者进行的操作

但如果实在没有办法的话,这种做法应该也是可以接受的

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

不妨在 changeEpisode 的时候自动关闭 menu,竖屏时就自动切换tab到选集界面,在 video_page 发送请求应该不会触发 loading 转圈动画

在查看评论后全屏。发生分集切换时评论将被自动加载

不过 b 站看番貌似也是这个逻辑,这样操作后评论区不会有加载动画,其他操作比如在评论页面时自动切换选集都会有加载动画

RefreshIndicator 我的担心是用户会频繁刷新导致反复发送请求,虽说 bangumi 应该能承受(?

@Predidit
Copy link
Owner

如果 B 站也是这样的话,我们应该可以保持现在的行为?

@Predidit
Copy link
Owner

Predidit commented Feb 15, 2025

不过我们确实可以简单加一个 RefreshIndicator

现在在网络问题导致的加载失败时,使用者无法进行重试

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

如果 B 站也是这样的话,我们应该可以保持现在的行为?

现在的行为在某些情况下会缺失动画和加载失败提示,所以我提出可以关闭 menu 或自动切换 tab

// Query comments when tab is visible
// We need lazy load comments to avoid unnecessary API requests
// so we only query comments when the tab is visible rather than video is loaded
if (tabController.index == 1) {
infoController.queryBangumiEpisodeCommentsByID(infoController.bangumiItem.id, episode);
}

@Predidit
Copy link
Owner

如果你觉得现在的行为不妥的话,可以提交一个 PR。这部分的代码所有者是你,我尊重你的意见。

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

没有不妥,只是我时常会扣一些细枝末节,比如动画,UI,代码格式啥的(

@Predidit
Copy link
Owner

不是,你误解了我的意思。

因为这里的讨论比较细节,而且不是特别重要的模块,再加上代码的所有者是你而不是我。

我的意思是对这里的修改 feel free,不用特别征求我的意见,只要没有明显 bug 或是回归就可以了。

@ErBWs
Copy link
Contributor Author

ErBWs commented Feb 15, 2025

那就保持这样吧,我暂时也没有时间去做这样需要大量测试的工作

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