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

fix: #9998 MkNote.vue, MkNoteDetailed.vue で、特定のMFMによってフッターのボタンが押せなくなる #9999

Merged

Conversation

hapo31
Copy link
Contributor

@hapo31 hapo31 commented Feb 19, 2023

What

以下 Issue の修正。
#9998

footer 部の z-index を上げた。

Why

フッターのボタンが押せなくなる問題があったため。

Additional info (optional)

全体の z-index がどうなっているかを眺めて、MFMの表示より上になればOKだろうという観点で z-index: 1 を指定した。

@syuilo
Copy link
Member

syuilo commented Feb 19, 2023

これをやるとおそらく大半のMFMアートと呼ばれるものが死ぬことになりそう

@hapo31
Copy link
Contributor Author

hapo31 commented Feb 19, 2023

なるほど、やはり影響はあるのですね。
死ぬ恐れのあるMFMを共有頂ければ、なんとか死なない方向に修正を更に加えることは可能かもしれません。(もちろん僕の方でも探してみるつもりです)

まあその辺に影響がないように穏便にやるとすると、 こちら で挙げていただいている通り z-index をいじるほうという方向になりそうですね・・・。

それか、 isLong の判定をもう少し賢く出来るといいのかもしれません。
https://github.com/hapo31/misskey/blob/63df2c851e31a30dda3b7e9edefb2a97e4c1daa6/packages/frontend/src/components/MkNote.vue#L197

@hapo31 hapo31 changed the title #9998 fix(client): add overflow: clip; fix: #9998 MkNote.vue, MkNoteDetailed.vue で、特定のMFMによってフッターのボタンが押せなくなる Feb 19, 2023
This reverts commit c43226a.

Revert "fix(client): add `overflow: clip;`"

This reverts commit c722515.
@hapo31
Copy link
Contributor Author

hapo31 commented Mar 1, 2023

こちらのPR、しばらく放置状態にしており大変恐縮です。
個人的な好みで z-index を指定する以外の方法を模索しましたが、
結局 @KawaneRio さんご指摘の通り z-index を上げる方向が最もシンプルでした。

以下に試行錯誤を一応残しておきます。

pointer-events: none;MkMisskeyFlavoredMarkdown コンポーネントのルート要素に指定し、リンクや blur, emoji などのマウスイベントを拾う必要のある要素に対しては個別に pointer-events: all; を指定する。

確かに動くが、今後他にクリック可能な要素を増やす際に pointer-events: all; を指定する必要がある。
コメントにコーディングルールとして記述しておくのもありかもしれないが、そもそもそのような設計は好ましくないと考える。(泥臭さを許容するならありかも?)

折りたたむかどうかの判定をMFM全体の要素の大きさを基準として行うようにする。

要は「 isLong の判定を賢くする」の実装だが、DOM操作を伴うため処理負荷の観点から見送り。
また、マウント後に行う必要があるのでガクガクしそう。

CSS で要素の大きさを制御し、 MkMisskeyFlavoredMarkdown 自体に折りたたむかどうかの機能を持たせる

これを実現するためには現在折りたたむかどうかを握っているのが親の MkNote 側であるため処理全体をリファクタリングする必要があり、そもそも不具合修正とリファクタリングを同時にやるべきではないと判断したため見送り。

@codecov
Copy link

codecov bot commented Mar 1, 2023

Codecov Report

Merging #9999 (e13ee01) into develop (8c88365) will increase coverage by 2.02%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #9999      +/-   ##
===========================================
+ Coverage    23.14%   25.17%   +2.02%     
===========================================
  Files          699      705       +6     
  Lines        64785    65273     +488     
  Branches      1984     2329     +345     
===========================================
+ Hits         14995    16431    +1436     
+ Misses       49790    48842     -948     
Impacted Files Coverage Δ
packages/backend/src/core/WebhookService.ts 39.08% <0.00%> (-1.40%) ⬇️
...ges/backend/src/core/activitypub/ApInboxService.ts 18.46% <0.00%> (-0.06%) ⬇️
packages/backend/src/types.ts 100.00% <0.00%> (ø)
packages/backend/src/misc/schema.ts 0.00% <0.00%> (ø)
packages/backend/src/server/ServerService.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/endpoints.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/stream/types.ts 0.00% <0.00%> (ø)
packages/backend/src/server/FileServerService.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/ApiCallService.ts 0.00% <0.00%> (ø)
packages/backend/src/server/api/EndpointsModule.ts 0.00% <0.00%> (ø)
... and 68 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@tamaina
Copy link
Contributor

tamaina commented Apr 12, 2023

@syuilo ping

@syuilo
Copy link
Member

syuilo commented Apr 13, 2023

今日中に対応する

@syuilo syuilo merged commit dffefda into misskey-dev:develop Apr 13, 2023
@syuilo
Copy link
Member

syuilo commented Apr 13, 2023

🙏🏻

na2na-p pushed a commit to na2na-p/misskey that referenced this pull request May 10, 2023
…のボタンが押せなくなる (misskey-dev#9999)

* fix(client): add `overflow: clip;`

* fix(client): add `overflow: clip;`

* Revert "fix(client): add `overflow: clip;`"

This reverts commit c43226a.

Revert "fix(client): add `overflow: clip;`"

This reverts commit c722515.

* feat(client): add z-index to .footer
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
packages/frontend Client side specific issue/PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants