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

Preview fixes #4199

Merged
merged 7 commits into from
May 28, 2023
Merged

Preview fixes #4199

merged 7 commits into from
May 28, 2023

Conversation

imshixin
Copy link
Contributor

fix #4197
fix #4177
使评论区上方的黄色小喇叭可选择性移除

show-cover代码里,移除了bpx-video的判断,在bpx视频下也能生效,所以就删了,不知有没有啥不好的地方

@imshixin imshixin marked this pull request as ready for review May 15, 2023 04:33
@imshixin
Copy link
Contributor Author

imshixin commented May 19, 2023

等等,我发现点问题。在封面加载出来之前点击播放,暂停后封面就会显示出来

@the1812
Copy link
Owner

the1812 commented May 20, 2023

要不先撤掉 #4197 的, 我来修那个

@imshixin
Copy link
Contributor Author

可以

@Geeyun-JY3
Copy link

Geeyun-JY3 commented May 20, 2023

有个问题,文档里介绍简化评论区的这个「删除评论区顶部的横幅」经实测就是指评论区上方的黄色小喇叭横幅。

- 删除评论区顶部的横幅

加进删除广告我赞成,不过简化评论区有些代码也应该跟着去掉了。


碎碎念:
前段时间就有想到提议不再让简化评论区来屏蔽,而统一让删除广告来屏蔽掉评论区黄色小喇叭横幅(毕竟动态评论区也有:#4163 (comment)),就是我当时不是很确定简化评论区是否能屏蔽,一直等不到横幅出现来验证我就一直没提。
今天终于等到了,实测简化评论区目前能在旧版播放页屏蔽黄色小喇叭横幅,但并不能在新版播放页屏蔽。
2022-06-02 实测发现简化评论区原来就已经能屏蔽动态评论区的黄色小喇叭横幅。

This reverts commit bae0eb3.
@imshixin
Copy link
Contributor Author

imshixin commented May 20, 2023

原来简化评论区也有这个代码,确实可以考虑删除
但是不知道以前的代码该删哪儿好

@the1812
Copy link
Owner

the1812 commented May 22, 2023

可以先放在 删除广告 里, 等我做了 简化评论区 各项功能的独立开关后再移动

@the1812
Copy link
Owner

the1812 commented May 22, 2023

SCSS 的变更是不是也手滑撤掉了(

@imshixin
Copy link
Contributor Author

😑手滑了(

@Geeyun-JY3
Copy link

可以先放在 删除广告 里, 等我做了 简化评论区 各项功能的独立开关后再移动

这么说关联一下 #2381

@the1812 the1812 merged commit 40b5295 into the1812:preview-fixes May 28, 2023
1 check passed
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.

None yet

3 participants