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

[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 #2294

Merged
merged 19 commits into from
Nov 12, 2024

Conversation

mtdkei
Copy link
Contributor

@mtdkei mtdkei commented Nov 7, 2024

チケットへのリンク / 変更の理由(元のissueがあればリンクを貼り付ければOK)

#2292

どういう変更をしたか?

  • 初期状態を確認し、目次が閉じている場合にはボタンテキストを「OPEN」、目次が開いている場合には「CLOSE」に設定するロジックを追加しました。
  • ボタンのクリック後に、チェックボックスの状態に応じてボタンテキストを即座に更新し、目次の状態と表示テキストが常に一致するようにしました。

スクリーンショットまたは動画

変更前 Before

2024-11-07.10.42.58.mov

変更後 After

2024-11-07.10.40.18.mov

実装者の確認事項

実装者はレビュワーに回す前に以下の事を確認してチェックをつけてください。

  • 複数の意図の変更 ( 機能の不具合修正 + 別の機能追加など ) を含んでいないか?
  • Files changed (変更ファイル)の内容は目視で確認したか?
  • readme.txt に変更内容は書いたか?
  • 本当にちゃんと確認をしたか?

プログラムの変更の場合

テストを書かないのは普通ではありません。書けるテストは極力書くようにしてください。

  • 書けそうなテストは書いたか?
    →view.jsのみの修正のためスキップ

変更内容について何を確認したか、どういう方法で確認をしたかなど

新規、既存の目次ブロックで以下を確認しました。
なお、目次ブロックは編集画面で表示タイプ > 初期表示状態「CLOSE」になっていることが前提です。

  • フロントエンドにある目次のOPENボタンをクリック後、テキストが適切に OPEN→CLOSE に切り替わることを確認。
  • 2回目以降のクリックでも同じようにCLOSE→OPEN→CLOSE→OPENと変わることを確認。
  • OPENをクリックした時に目次が表示されることを確認。

また、今回の変更により他の目次機能に影響がないことを確認済みです。

  • 表示タイプ > 初期表示状態「OPEN」でもフロントエンドでCLOSE→OPENと切り替わることや目次が閉じることを確認。
  • 編集画面でOPEN/CLOSEボタンが適切に変わることを確認。
  • 表示タイプ > スタイル「枠なし」時のOPEN/CLOSEボタンが適切に変わることを確認。

レビュワーに回す前の確認事項

  • 実装者はこのテンプレートのチェック項目をちゃんと確認してチェックしたか?

レビュワー確認方法・確認内容など

実装者と同じ確認を行ってください。
なお、このプルリクを確認するときは、npm run buildで確認してください。
(編集画面でエラーになるため。このissueは #2273 に書いてあります。)


レビュワー向け

レビュワーが確認して変更が反映されていない場合の確認事項

レビューしてみて意図した動作をしない場合は再度ビルドするなど以下の項目を確認してください。

  • プルしたか?
  • ビルドしたか?
  • ビルドしたディレクトリは正しいか(別の開発環境のディレクトリを見ていないか)?
  • npm install したか?
  • composer install したか?
  • キャッシュをクリアして確認したか?

@mtdkei mtdkei changed the title [ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックで1回目のみCLOSE表記にならない現象を修正 【確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックで1回目のみCLOSE表記にならない現象を修正 Nov 7, 2024
@mtdkei mtdkei marked this pull request as ready for review November 7, 2024 02:09
@mtdkei mtdkei changed the title 【確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックで1回目のみCLOSE表記にならない現象を修正 【確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 7, 2024
@mtdkei mtdkei self-assigned this Nov 7, 2024
@MasayaMORIMOTO
Copy link
Collaborator

@mtdkei
確認しました。問題ありません。
2人目の方、お願いします。
( readme.txt のコンフリクトの解消はお願いします)

@MasayaMORIMOTO MasayaMORIMOTO changed the title 【確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 【2人目確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 9, 2024
@mtdkei
Copy link
Contributor Author

mtdkei commented Nov 10, 2024

@MasayaMORIMOTO
ご確認ありがとうございます!readme.txtのコンクリフトについての運用を確認しますので、お手数ですがこちらのスレッドをまたのぞいていただけたらと思います。(確認次第、どのような場合でも確認したreadme.txtのコンクリフトはこちらで解消しますのでご安心ください。)

@kurudrive @goutetsuguma
レビュー時にreadme.txt がコンフリクトしてる場合はレビュワーの方もコンクリフトを解消していただいてもいいような気がしたのですが合ってますか?(プルリクOpen→レビューの間に別のプルリクがマージされてコンクリフトになるケースは少なくないため。)
明文化されてなかったのでわかりませんが、過去のプルリクを見たときにそのような感じで運用されていたので上記のように理解していました。
こちらはっきりしたらNotionに書き足そうと思います。

@goutetsuguma
Copy link
Contributor

goutetsuguma commented Nov 10, 2024

レビュー時にreadme.txt がコンフリクトしてる場合はレビュワーの方もコンクリフトを解消していただいてもいいような気がしたのですが合ってますか?(プルリクOpen→レビューの間に別のプルリクがマージされてコンクリフトになるケースは少なくないため。)
明文化されてなかったのでわかりませんが、過去のプルリクを見たときにそのような感じで運用されていたので上記のように理解していました。

@mtdkei @kurudrive

→ readme.txtのコンフリクト解消に関しては正式な運用として決まっていないような気がしますが(違っていましたらすみません!)、レビュー中に別のプルリクがマージされてreadme.txtのコンフリクトが発生するケースは多いため、レビューを進めやすくするために主にマージするレビュワー(2人目確認のレビュワーだけど、プルリクやリポジトリによっては1人目の場合もある)がマージ時にコンフリクトを解消している気がしますので、そのような運用で良いと思いました!

@mtdkei
Copy link
Contributor Author

mtdkei commented Nov 11, 2024

@goutetsuguma
ご返信ありがとうございます。とりあえずnotionに追記しました。

@MasayaMORIMOTO (CC: @akito-38 )
今回のように、ブランチの公開からレビューの間に、他のブランチがマージされることがあります。そのため、確認するブランチで主にreadme.txtがコンクリフトになります。
その場合はお手数ですが、今後、実装者だけでなくレビュワーの方でコンクリフトを修正することも可能ですので、お気づきの時は解消していただけたらと思います。特に2人目のレビュワーがマージする際はレビュワー自身でreadme.txtのコンクリフトを解消してください。
実装者でないと難しいコンクリフトを起こしている場合はメンションしていただけたらと思います。

@sysbird sysbird changed the title 【2人目確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 【2人目確認中】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 12, 2024
@sysbird
Copy link
Member

sysbird commented Nov 12, 2024

@mtdkei
まず develp で再現しない状況です
状況は異なりますが不具合はありますね
デモサイトでの再現は確認できてます

私のローカルでは下記の状況で、どちらも不具合あるように見えます

develop ブランチ

develop

このブランチ

current

@sysbird sysbird closed this Nov 12, 2024
@sysbird
Copy link
Member

sysbird commented Nov 12, 2024

また、目次ブロック設置時のデフォルト表示が異なるようです
意図的でしょうか?
##develop ブランチ
開いている
dev-default

##このブランチ
閉じているが、ボタンのテキストが逆
cur-default

@sysbird sysbird reopened this Nov 12, 2024
@sysbird
Copy link
Member

sysbird commented Nov 12, 2024

私がうまく確認できてないと思いますので
@goutetsuguma さんかどなたか見てもらえると助かります

@sysbird sysbird changed the title 【2人目確認中】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 【2人目確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 12, 2024
@goutetsuguma
Copy link
Contributor

goutetsuguma commented Nov 12, 2024

@mtdkei

また、目次ブロック設置時のデフォルト表示が異なるようです
##このブランチ
閉じているが、ボタンのテキストが逆

確認しましたところ鳥さんのコメントの症状が確認できました。
編集画面で設置した時に初期状態がOPENだけど、編集画面では閉じてしまっているようでした。フロント画面では問題ありませんでした。
元々の不具合の、初期状態がCLOSE時の表記は治っていました。

スクリーンショット 2024-11-12 13 36 32

調整待ちとさせていただきました🙇

@goutetsuguma goutetsuguma changed the title 【2人目確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 【調整待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 12, 2024
@mtdkei
Copy link
Contributor Author

mtdkei commented Nov 12, 2024

@goutetsuguma @sysbird
ご確認いただきありがとうございます。
今一度確認しましたが自分の環境では developで issue #2292 の状態を確認でき、さらにこのブランチで編集画面に目次ブロックを設置した時には以下のようになります。
ですので、調整待ちとしていただいていますが、どこでそのようになっているのかが特定できていません。すみません。もう少し確認してみます。

すみません。別の環境で確認したら確認できました。なぜ最近になってここまで異なってしまったのかが不明でちょっと困惑してます。。直します。
なお、このissueをOpenした時点、また、環境によっては以下になります。

image
image

@mtdkei mtdkei changed the title 【調整待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 【調整中】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 12, 2024
@mtdkei
Copy link
Contributor Author

mtdkei commented Nov 12, 2024

@sysbird @goutetsuguma
修正しました。おそらく「編集画面で設置した時に初期状態がOPENだけど、編集画面では閉じてしまっている状態」が解消されていると思います。
こちらではちょっと環境によってばらつきがあり、まっさらな状態で確認したところOKだったのですが、今一度ご確認いただけたらと思います。

@mtdkei mtdkei changed the title 【調整中】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 【2人目確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 12, 2024
@sysbird
Copy link
Member

sysbird commented Nov 12, 2024

@mtdkei
対応ありがとうございます!
再度見てみました
編集画面でClose、Oopen どちらのときもフロントでは開いた状態(設定ではOPEN?)で表示されるようです

環境によるでしょうか?
いちおう、

  • WP 6.6 / 6.7
  • FireFox / Chrome
  • キャッシュクリア
    で確認しました

@mtdkei
Copy link
Contributor Author

mtdkei commented Nov 12, 2024

@sysbird
ご確認ありがとうございます。複数で確認してみましたが、こちらの環境ではそのような状態にはなりませんでした。

久納さんのレビューにある

編集画面で設置した時に初期状態がOPENだけど、編集画面では閉じてしまっているようでした。フロント画面では問題ありませんでした。

はこちらでも確認できたのでそちらを修正しました。何か再現できる状態があればそちらで確認させていただきます。

@goutetsuguma
Copy link
Contributor

goutetsuguma commented Nov 12, 2024

@mtdkei @sysbird
わたしも確認しまして、「編集画面で設置した時に初期状態がOPENだけど、編集画面では閉じてしまっている状態」が解消されていました。

編集画面でClose、Oopen どちらのときもフロントでは開いた状態(設定ではOPEN?)で表示されるようです

鳥さんのコメントですが、私の環境では、初期状態がCloseの場合は、フロント画面で閉じた状態で正常に動くことが確認できました。

WP 6.6でChromeでみていますが、なんでしょうね?なにかの設置の違いかな

@sysbird
Copy link
Member

sysbird commented Nov 12, 2024

@mtdkei @goutetsuguma @MasayaMORIMOTO
私の環境ではもともと再現の状況が異なってますので、
森本さんさんとくのうさんがOKということですので、マージでよろしくお願いします

@goutetsuguma goutetsuguma changed the title 【2人目確認待ち】[ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 [ 目次 ] 初期表示状態CLOSE時、フロント画面でOPENボタンをクリックすると1回目のみCLOSE表記にならない現象を修正 Nov 12, 2024
@goutetsuguma goutetsuguma merged commit 55c1805 into develop Nov 12, 2024
14 checks passed
@goutetsuguma goutetsuguma deleted the fix/toc/button-initial-toggle branch November 12, 2024 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants