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.

Sign up for GitHub

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

Jump to bottom

enhance: ノート編集の際のノート作成画面を改善 #11947

Closed
wants to merge 2 commits into from
Closed

enhance: ノート編集の際のノート作成画面を改善 #11947

wants to merge 2 commits into from

Conversation

kakkokari-gtyih
Copy link
Contributor

@kakkokari-gtyih kakkokari-gtyih commented Oct 1, 2023 •
edited
Loading

What

Related to #11944 #11945

  • ノート編集の際に変更できない項目を選択できなくする
  • ノート編集の際のノート作成画面の「ノート」ボタンを「保存」にする
  • 連合ありのノートを編集する際には確認ダイアログを追加

Why

現在の投稿画面はノート新規作成時と同一であり混乱が生じるため

Additional info (optional)

Checklist

  • Read the contribution guide
  • Test working in a local environment
  • (If needed) Add story of storybook
  • (If needed) Update CHANGELOG.md
  • (If possible) Add tests

@github-actions github-actions bot added packages/frontend Client side specific issue/PR packages/backend Server side specific issue/PR labels Oct 1, 2023
@codecov
Copy link

codecov bot commented Oct 1, 2023 •
edited
Loading

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (5fd0cb3) 79.22% compared to head (63389cd) 79.33%.

Additional details and impacted files 
@@             Coverage Diff             @@
##           develop   #11947      +/-   ##
===========================================
+ Coverage    79.22%   79.33%   +0.11%     
===========================================
  Files          931      753     -178     
  Lines        99481    76182   -23299     
  Branches      8003     7494     -509     
===========================================
- Hits         78809    60441   -18368     
+ Misses       20672    15741    -4931
Files Coverage Δ
...s/backend/src/server/api/endpoints/notes/update.ts 69.69% <0.00%> (-7.84%) ⬇️

... and 180 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tai-cha
tai-cha previously approved these changes Oct 1, 2023
View reviewed changes
Copy link
Contributor

@tai-cha tai-cha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

私的には一旦これでいいと思います（ちょうどいい対応だと思う）

@kakkokari-gtyih
Copy link
Contributor Author

kakkokari-gtyih commented Oct 2, 2023 •
edited
Loading

@syuilo いかがでしょう？
次のリリースに #11945 を含めない（#11945 を待たない）のならマージすべきだとおもいます

@kakkokari-gtyih
Copy link
Contributor Author

マージするか

@syuilo
Copy link
Member

syuilo commented Oct 2, 2023

連合しないという警告を表示するのではダメかしら？

@syuilo
Copy link
Member

syuilo commented Oct 2, 2023

自分としては連合しなくてもいいから連合ありのノートを編集したい時がある

@tai-cha
Copy link
Contributor

tai-cha commented Oct 2, 2023 •
edited
Loading

自分としては連合しなくてもいいから連合ありのノートを編集したい時がある

これはわかる
警告が出るぐらいでいいかなと思っていたところは自分もあるけどどちらもメリットデメリットがあるのよね（）

@kakkokari-gtyih
Copy link
Contributor Author

kakkokari-gtyih commented Oct 2, 2023 •
edited
Loading

自分としては連合しなくてもいいから連合ありのノートを編集したい時がある

「よくわかってないなら編集しないでください」的な書き方にしてダイアログ作るか

【ノートの編集を行いますか？】

現時点ではノートの編集は連合されません。
外部サーバーのユーザーに編集内容が伝わらないので、重要な内容の修正の際はご注意ください。
この警告の内容がよくわからない場合は編集を行わないことをおすすめします。

など

@syuilo
Copy link
Member

syuilo commented Oct 2, 2023

ただおそらく次のリリースまでには連合が実装されそう

@tai-cha
Copy link
Contributor

tai-cha commented Oct 2, 2023 •
edited
Loading

#11936
↑
これの問題もあるので投稿画面自体をupdateの時に挙動変えられたらうれしいなとちょっと思う（Updateな場合のみに注意を出したりボタンを無効化できたり）

@tai-cha
Copy link
Contributor

tai-cha commented Oct 2, 2023

連合の実装はするけど最初は編集可能項目今と同じ最小限になるとおもうので

@kakkokari-gtyih
Copy link
Contributor Author

ただおそらく次のリリースまでには連合が実装されそう

これの問題もあるので投稿画面自体をupdateの時と挙動変えられたらうれしいなとちょっと思う

ならこのPRでは投稿画面の実装をやっておきましょうかね

@kakkokari-gtyih
Copy link
Contributor Author

それかPR作り直したほうがいいか

@kakkokari-gtyih
Copy link
Contributor Author

Approveが出ててしまっているのでPR作り直します

@tai-cha

This comment has been minimized.

Sign in to view
@acid-chicken acid-chicken reopened this Oct 2, 2023
@acid-chicken acid-chicken dismissed tai-cha’s stale review October 2, 2023 11:40

Dismiss 機能使ってください

@kakkokari-gtyih kakkokari-gtyih changed the title (fix)【暫定処置】ノート編集機能を連合なしでのみ使用できるように enhance: 投稿の編集の際のノート編集画面を改善 Oct 2, 2023
@kakkokari-gtyih kakkokari-gtyih marked this pull request as draft October 2, 2023 11:41
@kakkokari-gtyih kakkokari-gtyih changed the title enhance: 投稿の編集の際のノート編集画面を改善 enhance: ノート編集の際のノート作成画面を改善 Oct 2, 2023
@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

テキストとCWだけ編集できれば良いから専用のコンポーネント作る方が楽そう

@tai-cha
Copy link
Contributor

tai-cha commented Oct 4, 2023

テキストとCWだけ編集できれば良いから専用のコンポーネント作る方が楽そう

今後編集可能項目増やしたりとかはないんですかね？（ファイルとか）
まあ今早くこれを載せるなら早くできるのはこれというのは間違いないと思うんですが

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

テキストとCWだけ編集できれば良いから専用のコンポーネント作る方が楽そう

今後編集可能項目増やしたりとかはないんですかね？（ファイルとか） まあ今早くこれを載せるなら早くできるのはこれというのは間違いないと思うんですが

可能性はあるけど実装コストを考えると低そう

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

機能自体キャンセルする可能性が高まってきたので作業はストップしていただけると🙏

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

一旦ノート編集機能は撤回されました 🙏🏻

@syuilo syuilo closed this Oct 4, 2023
@kakkokari-gtyih
Copy link
Contributor Author

どうして（）

@syuilo
Copy link
Member

syuilo commented Oct 4, 2023

実装が難しいのと、実装したとしてコストに見合う価値がないと判断しました

@kakkokari-gtyih
Copy link
Contributor Author

なるほど

@kakkokari-gtyih kakkokari-gtyih deleted the notes-edit-workaround branch March 18, 2024 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tai-cha tai-cha tai-cha left review comments

Assignees
No one assigned
Labels
packages/backend Server side specific issue/PR packages/frontend Client side specific issue/PR
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@kakkokari-gtyih @syuilo @tai-cha @acid-chicken