-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
enhance(frontend): 投稿フォームの設定メニューを改良 #14804
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #14804 +/- ##
============================================
- Coverage 40.31% 20.06% -20.26%
============================================
Files 1564 731 -833
Lines 198083 104325 -93758
Branches 3837 1199 -2638
============================================
- Hits 79863 20933 -58930
+ Misses 117617 82839 -34778
+ Partials 603 553 -50 ☔ View full report in Codecov by Sentry. |
コンフリクト解消 |
コンフリクト解消 |
コンフリクト解消 |
コンフリクト解消 |
コンフリクト解消 |
コンフリクト解消 |
コンフリクト解消 |
/deploy |
/preview |
コンフリクト解消 |
mkmenuと共通化すると実装のシンプルさが失われるから https://github.com/misskey-dev/misskey/blob/develop/packages/frontend/src/components/MkVisibilityPicker.vue みたいに完全オリジナルで良い気がしてきている |
#14804 (comment) で分けたんだけどこのやり方じゃちょっとまずかった感じかしら |
そーねーコンポーネントの設計に親子関係がある場合、他のコンポーネントがその子を参照するとややこしくなる気がする(子は親に隠蔽されているべき) |
そういう前提を崩さず、かつ子を参照しない上手い共通化の方法は自分は思いつかなかった |
@syuilo MkMenuの上部にslotを設けてMkPostFormOtherMenuからMkMenuを参照させる→そのslotに文字数カウントを入れるとかどうかしら |
うーむアリっちゃアリだけど任意の位置に任意のコンポーネントを入れたい場合には対応できず、今回のようなたまたま入れやすい位置に入れたいものがある場合しか対応できないから完全な解決策ではないわね あとデザインを見る感じ今回に関してはそこまで共通化するメリットなさそうだからやっぱ #14804 (comment) だと思う |
あーまてよ |
今後設定メニューに追加の項目が生まれる可能性があることを考えると共通化のメリットは大きくなってくるな |
うーーーーーーーーーーーーーーーーむ |
ポップアップメニューの役割上、中腹でべつのコンポーネントを入れるケースがあまり思いつかない(最上部か最下部かが主だと思う)のでそれは必要になったときに考えるので良い可能性はある |
中腹はMenuItemに type MenuSlot = {
type: 'slot';
name: string;
} 今回の文字数カウントはMenuの部分とは完全に分離した場所に置いているものなので少し違うかも(type: |
あーでもこれでは |
What
Why
Fix #14794
Fix #10785
Additional info (optional)
Checklist