misskey icon indicating copy to clipboard operation
misskey copied to clipboard

feat(frontend): ノートを畳むときの計算をレンダーしたときの高さ基準にする

Open KisaragiEffective opened this issue 1 year ago • 18 comments

Summary

ノートを畳むときの計算をレンダーしたときの高さ基準にしてほしい

Purpose

現状の実装だと、とてもとても長い一行のコードブロックがたたまれるのに何回か改行を連打したノートがたたまれないなど^1、一貫性がないように思う ↑「畳む」が存在する意義は「高すぎるとうるさい」という認識だが、文字列長でfalse-positiveやfalse-negativeが起きてしまっている

Do you want to implement this feature yourself?

  • [ ] Yes, I will implement this by myself and send a pull request

KisaragiEffective avatar Feb 12 '24 09:02 KisaragiEffective

改行を連打したノートがたたまれない

改行自体には上限があったと思います。(10行以上で畳まれる: https://github.com/misskey-dev/misskey/blob/b95e25004f87cf0743f4a363cd5ed4a7720ad27d/packages/frontend/src/scripts/collapsed.ts ) 一方、畳まれないものの分かりやすい例としては、横に長い絵文字(名前が短いもの)などがあると思います。 畳まれる条件の文字数に達するまでに、相当な数の絵文字が表示出来るため、本来畳まれるような幅でも畳まれないと記憶しています。

Sayamame-beans avatar Feb 12 '24 09:02 Sayamame-beans

パフォーマンス面とのトレードオフが気になるところではあるかも?

kakkokari-gtyih avatar Feb 12 '24 10:02 kakkokari-gtyih

もちろん、そんなに負荷のかからない処理ならそうしたほうがいいとは思っている

kakkokari-gtyih avatar Feb 12 '24 10:02 kakkokari-gtyih

なんかいい感じにcssのmax-heightと組み合わせてできないかなぁとは結構前から思ってはいます。具体的な調査はしてない

anatawa12 avatar Feb 12 '24 10:02 anatawa12

いや、でもな… 端末によってノートが隠れる基準が変わってしまうのがMisskeyノートのテクニカルな部分の変更に当たる(既存のMFMテクニックやプラクティスが通用しなくなる)のでまずいかもしれないのと↓ https://misskey.io/notes/9pmb5deir91r0220 を両立するとなると一文字入力するごとに高さ計算が入るのでちょっとしんどそうというのがある

kakkokari-gtyih avatar Feb 12 '24 10:02 kakkokari-gtyih

(個人的には https://misskey.io/notes/9pmb5deir91r0220 を実装するにとどめてもいいんじゃないかなと思っています)

kakkokari-gtyih avatar Feb 12 '24 10:02 kakkokari-gtyih

正直なところ短いx2が縮まるのが個人的にすごい違和感(イライラ)あるんですよね。

anatawa12 avatar Feb 12 '24 10:02 anatawa12

正直なところ短いx2が縮まるのが個人的にすごい違和感(イライラ)あるんですよね。

もうちょっと畳む基準を複雑にする(MFMのASTを投げて判定させる?)ようにすればいいのかも

kakkokari-gtyih avatar Feb 12 '24 10:02 kakkokari-gtyih

正直なところ短いx2が縮まるのが個人的にすごい違和感(イライラ)あるんですよね。

それでいうと、scaleの小さくする方向でも畳まれるのとか気になりますね… (別のissueにした方が良さそうですかね?) related: https://github.com/misskey-dev/misskey/issues/13264#issuecomment-1938070921

borderのパラメータを見てshouldCollapseできるようにしないといけなさそう

Sayamame-beans avatar Feb 12 '24 10:02 Sayamame-beans

畳まれる領域のrefを取っておいて、onMountedイベントでclientHeightを計測して判定するようにすれば可能だと思います。

端末によってノートが隠れる基準が変わってしまうのがMisskeyノートのテクニカルな部分の変更に当たる

に関してはその通りですが、個人的には各々で隠す高さを変える設定ができるようになったらそちらの方が嬉しいと思います。

FineArchs avatar Feb 13 '24 09:02 FineArchs

正直なところ短いx2が縮まるのが個人的にすごい違和感(イライラ)あるんですよね。

もうちょっと畳む基準を複雑にする(MFMのASTを投げて判定させる?)ようにすればいいのかも

これ実現できたかもしれない

kakkokari-gtyih avatar Apr 28 '24 11:04 kakkokari-gtyih

暫くは仮想行数と高さ計測の両方を設定から選べるようにするのも手かと思います

FineArchs avatar May 09 '24 21:05 FineArchs

暫くは仮想行数と高さ計測の両方を設定から選べるようにするのも手かと思います

(実装した人間が言うと元も子もないかもしれないけど)普通にheightだけの判定でよさそう 仮想行列をいちいちカウントしてたら重くなる

kakkokari-gtyih avatar Jun 10 '24 00:06 kakkokari-gtyih

仮想行列をいちいちカウントしてたら重くなる

これそんなに重くなりますかね? MFMのパースよりは軽そうに見えます

FineArchs avatar Jun 10 '24 01:06 FineArchs

MFMのパースよりは軽そう

MFMをパースする行為と行数を算出する行為との負荷の比較ではなくて、行数を算出する行為とheightを取得する行為との比較について言いたかった

あと、行数算出はコードが長すぎるという問題もある

kakkokari-gtyih avatar Jun 19 '24 15:06 kakkokari-gtyih

横ですが JavaScriptでレンダリングされたDOMから値取ってくるのは比較的重い処理なので(仮想DOMのレンダリングよりReadが多い分重いはず)ノートの高さ計算のように大量に計算が発生するようなシチュエーションだと危険かもしれない どちらかというとmaxheight指定でoverflow:hiddenにして、borderの下の方をblurかなんか(blurもSafariだと重いみたいな罠があるんだけも)で隠すとかのほうが軽い気がする

いずれの実装にするにせよ、可能ならiPhoneくらいのスペックにスロットリングした(できれば)Safariで過去のノートに遡ってもパフォーマンスに明らかな劣化がないか確認してもらえると安心です

fruitriin avatar Jun 20 '24 00:06 fruitriin

ちなみにMFMのパースはJavaScriptで完結するぶん、レンダリングサイクルの影響を受けないという点でコードの見た目より高速になるような気がします (ベンチ測ってないので憶測で言っています

fruitriin avatar Jun 20 '24 00:06 fruitriin

正直ノートが多かったりした時にどの程度パフォーマンスに影響が出るのか分からないので、実験的に投入したい気持ちがあります。 ダメそうだったら設定で戻す感じで…

FineArchs avatar Jun 20 '24 12:06 FineArchs