gaogaotiantian / biliscope

Bilibili chrome extension to show uploader's stats
MIT License
594 stars 46 forks source link

Add history to recommended video on the homepage #171

Closed Natrium0521 closed 3 months ago

Natrium0521 commented 4 months ago

image RT,为首页右侧的换一换功能添加历史记录

gaogaotiantian commented 4 months ago

Overall的想法是ok的,我也知道有插件在做这个东西,这东西也挺实用。相对来说设计看起来挺独立的所以我们可以加一下(尽管下次还是提个issue先比较好)。

你这里设计有个地方我觉得有点问题,就是你用了两个stack来做翻页的数据保存,导致你每次翻页的时候都要pop+push。这里用一个array去存就行了啊,你只需要改index就好了,完全没必要来回来去pop push。

你也不需要做那个巨复杂的是不是要保存的操作,用一个flag表示现在的video cards有没有被保存过就行了,也容易读。你那一大串读起来可是太费劲了。这样你也不用担心在回溯过程中按换一换时候数据重复的问题了,保留到index为止的数据然后换就完了。

对于有功能的button,我还是习惯用id定位。id加上一个前缀保证不和B站原有的东西命名冲突就好了。

“上一页”,“下一页”感觉在UI上稍显啰嗦,我觉得这里可以考虑用前后箭头的图标,svg的不需要去CORS的那种。

如果你本身要找roll-btn的parent,为啥不直接找他parant呢?你在addButton里又做了一个query,而且这个query没检查(逻辑上这里必然有,但是如果只看local是看不出来的)。如果是我,我会在load里call的时候,直接找那个.feed-roll-btn,然后把它作为argument传到addButton里,你就可以直接在里面加了。

还有一点点styling上的事情,我们可以等逻辑整理的差不多了再说。

Natrium0521 commented 4 months ago

你这里设计有个地方我觉得有点问题,就是你用了两个stack来做翻页的数据保存,导致你每次翻页的时候都要pop+push。这里用一个array去存就行了啊,你只需要改index就好了,完全没必要来回来去pop push。

用两个栈是参考浏览器的前进后退,只用一个array的话在前进和后退的边界判断上有些麻烦吧,用两个栈表述感觉更简单明了,而且只有10个左右的feed-card,对性能没有很大影响。

用一个flag表示现在的video cards有没有被保存过就行了

这里的问题是点击换一换之后貌似没有什么优雅的方法判断是否刷新过了,比如连续点击被节流了,所以我的方案是栈中直接不保存现在展示的cards,在切换的时候再做处理。

“上一页”,“下一页”感觉在UI上稍显啰嗦,我觉得这里可以考虑用前后箭头的图标,svg的不需要去CORS的那种。

是只用图标表示吗

gaogaotiantian commented 4 months ago

用两个栈是参考浏览器的前进后退,只用一个array的话在前进和后退的边界判断上有些麻烦吧,用两个栈表述感觉更简单明了,而且只有10个左右的feed-card,对性能没有很大影响。

Okay两个stack的实现也行吧。边界判断没啥难的因为就是查下index而已,清空的时候可能稍显复杂。

这里有两个细节。第一,你backward和forward俩函数几乎长得一样。我觉得你应该做一个function replaceFeedCards(feedCards),这个函数把现在主页的feedcard都换成给的这个feedcards,然后返回被换下来的那些。这样你的backward和forward应该大概只要四五行了。

第二,在查重的时候,应该不需要这么复杂(而且你现在依赖我们自己产生的biliscope-videoid,这不是个大问题,但是能摆脱最好),你应该可以直接比较node,oldFeedCards[0] == this.backwardStack.at(-1)[0]就应该可以了。这里做个注释,说明一下为啥要干这件事。

文件最后加个回车,好习惯。

是只用图标表示吗

对,我感觉两个和换一换上面那个刷新图标一样大的左右箭头就足够了,这两个东西应该表达能力是足够强的。

然后相比起disable这个按钮,我觉得直接d-none可能更舒服。

Natrium0521 commented 4 months ago

display: none 不占空间,比如一直点回退到最后回退按钮消失,会点到前进,所以用 visibility: hidden ,以及我在想这两个按钮会不会一个在换一换上面,一个在下面更好一点?

加了个 feedcardBatchSize 是修复之前因为一些操作可以让DOM里的feed-card一直增加,虽然多余的并不会显示在页面上。

gaogaotiantian commented 4 months ago

不要去动它原来那个按钮的位置,如果一个上一个下就会动原来的按钮或者出parent div了。可以visibility hidden。我主要是想在大部分情况下(不用这个功能的时候)界面只多一个向前的按钮。

我没懂你batch的逻辑,不明白这是要干什么。如果每个stack的size可能不一样,那你这样不是在丢数据么?你在每一次前后操作的时候都只留了batchsize的数据,如果未来batchsize大了你就补不全了啊,你这数据会越来越少啊。

什么操作会让隐形card越来越多?是换一换按钮B站自己的功能会出问题?还是你写的部分会出问题

Natrium0521 commented 4 months ago

什么操作会让隐形card越来越多?

之前的版本按照 换一换 换一换 后退 后退 前进 这五个一组循环的话会触发。

分析了一下,首先假设一次刷新10条。换一换应该自己存了一个feedcard的列表,每次刷新是把列表中的移除再换成新的,如果我们在换一换之前回退过,也就是说当前页面上feedcard的并不是他自己存的列表,那么他并不会帮我们删除feedcard,而是直接添加新的feedcard。

image

添加的位置是插入到图中第二个框之前。

而我们插入的位置是在第二个框之后。如果这里不做处理限制入栈的大小,后退时会把他刷新的和本应被删掉却没有删掉的一起入栈,也就是说入了20个feedcard。此时再前进会插入20个feedcard,虽然这20个中有10个是被换一换保存的,刷新可以删掉,但如果是两次换一换加两次后退,那么这20个在刷新的时候就都不会被删除了,之后DOM中就有了30个feedcard,以上面的操作循环会不断增多。

所以我的解决方案是在入栈的时候只入前10个,虽然在第二个框之后有时候会多出来10个feedcard,但因为多余元素不入栈了,所以影响不大。

如果每个stack的size可能不一样,那你这样不是在丢数据么?

理论上初始有几个feedcard就刷新几个,数量不变的

gaogaotiantian commented 4 months ago

为什么不在换一换的event hook里手动删掉我们插进去的card?

Natrium0521 commented 4 months ago

有什么好的删除时机吗,监听dom?

gaogaotiantian commented 4 months ago

我们现在不是有换一换的onclick么?然后需要删除的是已经存在的DOM,在那里直接删不行么?

Natrium0521 commented 4 months ago

加载新的feedcard是要时间的,直接删会有闪屏现象

gaogaotiantian commented 4 months ago

把要删掉的cards放到一个pendingRemoval里,每次按键触发remove呢?我觉得现在这个方法太tricky了,它不直接,读起来很费劲。我们想要的效果是该删的东西把它删掉,我们应该想办法去删掉这些没被删掉的DOM。

Natrium0521 commented 4 months ago

还是监听DOM吧,点击换一换开始监听,节点更新后触发回调,删除后移除监听

gaogaotiantian commented 3 months ago

还剩下两个小问题。那个箭头有预览图么发一下?

Natrium0521 commented 3 months ago

image

gaogaotiantian commented 3 months ago

有没有那种纯三角的箭头?我感觉那个好像更合适,就dvd里那个play那种,或者是B站视频暂停开始那个箭头,一个三角形往边上指的

Natrium0521 commented 3 months ago

image 这种?

gaogaotiantian commented 3 months ago

哎对的这种。你看一下实心和空心哪个好看一些。我不确定,但是形状是这个

Natrium0521 commented 3 months ago

感觉空心的简洁一点。大小要不要调整

gaogaotiantian commented 3 months ago

我觉得和那个刷新一个size会不会显得统一一些

Natrium0521 commented 3 months ago

image 微调了一下

gaogaotiantian commented 3 months ago

这个可以了,以后觉得别扭再调就行。

gaogaotiantian commented 3 months ago

感谢贡献~这个功能本身还是挺好的。一个小建议,下次不要在自己的master上做feature,否则你再同步是很麻烦的事情。新开一个feature branch做。