report_template.vue, report_template_modal.vueをreactに対応させる

komagata commented 2 years ago

下記を参考にしてreactに変える。

react導入 + 管理画面のコース一覧をreact化 by komagata · Pull Request #5515 · fjordllc/bootcamp

github-actions[bot] commented 2 years ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 1 year ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 12 months ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

github-actions[bot] commented 9 months ago

このissue|PRは60日間更新がないため7日後にcloseします。closeしたくない場合はstaleラベルを外してください。

junohm410 commented 9 months ago

@komagata CC: @machida

お疲れ様です。日報テンプレート機能のコンポーネントをReact化する件で、一点質問したいことがありご連絡します。

聞きたいこと

現状の「編集を保存せずにモーダルを閉じて、もう一度編集モーダルを開くと、先ほど編集をしていた文面がセットされている」という挙動は、維持したほうがいいでしょうか？

挙動のスクショ

liB80MvjVt

現状の仕様

現状のVueでは、下記のような処理で上記の動きを行っています。

編集中のテンプレートの入力内容をreport_template.vue（テンプレート登録・反映のボタンのコンポーネント）のeditingTemplateプロパティで保管。
それを子の編集画面のモーダルreport_template_modal.vueに渡す。

子のモーダルを閉じるcloseModalイベントハンドラの処理で、editingTemplateをモーダルを閉じる直前の編集状態に更新することで、次にモーダルを開けたときに、前回の編集状態から編集を再開することができるようになっています。

考えたこと

現在これをReactで書き換えていますが、これを実現するには、親のコンポーネントでeditingTemplatestateを持ち、それを子のモーダルに渡す必要があると考えています。

// ReportTemlateコンポーネント

export default function ReportTemplate({ templateDescription, templateId }) {
  // ...
  const [editingTemplate, setEditingTemplate] = useState(registeredTemplate)

// ...

{showModal && (
  <ReportTemplateModal
    // ...
    editingTemplate={editingTemplate}
    setEditingTemplate={setEditingTemplate}
    closeModal={closeModal}
  />
)}

// ReportTemplateModalコンポーネント

<textarea
  // ...
  value={editingTemplate} // 初期値にstateで保存している最新のテンプレートの下書きを設定
  onChange={(e) => setEditingTemplate(e.target.value)}></textarea>

いったんこのような形で作業を進めました。 https://github.com/fjordllc/bootcamp/pull/7164

しかしこの場合、子のモーダルでテンプレートの編集の入力をするたびに、stateを管理する親のボタンのコンポーネントまで再レンダリングされるので、パフォーマンス的に微妙なのではないか、と考えました。

また、あくまで個人の感覚ではあるのですが、そもそもこのモーダルを開いた時は、前回にモーダルを開いていたときの編集内容ではなく、編集のベースとなる「そのときに登録されているテンプレート」が常にセットされる方が自然なのではないかとも考えました。

https://github.com/fjordllc/bootcamp/issues/3391 https://github.com/fjordllc/bootcamp/pull/3419 この機能が追加されたときのIssue/PRも見ましたが、とくにこの挙動が指定されて実装された感じではなさそうでした。

考えてみた提案内容

編集中のテンプレートの入力内容は、子のモーダル内で管理する。親と切り離すことで不要な再レンダリングを起こさないようにする。
親での編集中のテンプレートの入力内容の管理をやめるので、モーダルを開くときは常に「登録されているテンプレート」がセットされる。
編集中に誤ってモーダルの外をクリックしてしまい、意図せず編集内容が失われることを防ぐために、「テンプレートの編集内容が今の登録されているものと異なるとき && モーダルの外をクリックして閉じるとき」だけ、「編集モーダルを閉じますか？」的なconfirmを入れる。

上記の動作イメージが下です。

1h8gq5xAv9

編集内容はモーダルを閉じても維持すべきかどうか以外に、

そもそもこの程度の再レンダリングはそこまで気にしなくても良い
するにしても、書き換えのIssueですべきことではない（別のIssueにわける）
親で編集内容のstateを持ち、渡された子でそのstateをセットしつつ、親側でレンダリングを回避するための手法について調べるべき方向性（refで頑張って書けるかなと思ったのですが、子側では編集内容によって登録ボタンをdisabled=trueするかどうかなどを動的に操作したいので、いずれにしてもeditingTemplateはリアクティブに管理はしたいイメージです）

以上はあくまで例ですが、このようなアドバイスがもしあればご教示いただけるとありがたいです🙏 お手数をおかけしますが、よろしくお願いいたします。

komagata commented 9 months ago

@junohm410

現状の「編集を保存せずにモーダルを閉じて、もう一度編集モーダルを開くと、先ほど編集をしていた文面がセットされている」という挙動は、維持したほうがいいでしょうか？

維持しなくて良いです。

そもそもこの程度の再レンダリングはそこまで気にしなくても良い

気にしなくて良いです。

junohm410 commented 9 months ago

@komagata ご確認いただき、ありがとうございました🙏

それでは、「編集を保存せずにモーダルを閉じて、もう一度編集モーダルを開くと、先ほど編集をしていた文面がセットされている」という挙動は維持せず、

編集中のテンプレートの入力内容は、子のモーダル内で管理する。親と切り離すことで不要な再レンダリングを起こさないようにする。

親での編集中のテンプレートの入力内容の管理をやめるので、モーダルを開くときは常に「登録されているテンプレート」がセットされる。

編集中に誤ってモーダルの外をクリックしてしまい、意図せず編集内容が失われることを防ぐために、「テンプレートの編集内容が今の登録されているものと異なるとき && モーダルの外をクリックして閉じるとき」だけ、「編集モーダルを閉じますか？」的なconfirmを入れる。

こちらで提案させていただいた内容でまずは実装し、メンバー・komagataさんにレビューいただくように進めさせていただきます。

また再レンダリングを気にする温度感的なものもシェアいただきありがとうございます。こちらは今後の参考にさせていただきます！引き続きよろしくお願いいたします。

junohm410 commented 9 months ago

@komagata @machida お疲れ様です。2023/12/27の開発MTG・質問タイムにて、上で提案させていただいた内容に加え、本Issueで下記の変更を含めることを確認させていただきました。 Issue上でも下のように記録しておきます。（本来は事前にIssue上で確認すべき内容でした。以後、気をつけます。）

編集内容が登録内容と同じ場合、「変更」ボタンが押せないように変更

変更前は、既存のテンプレートから無編集の状態でも「変更」ボタンを押してAPIを叩けていましたが、このシナリオでボタンを押せないように修正する予定です。

イメージ

「新規登録 -> ページを更新せず再度をモーダルを開き、再編集してテンプレート変更する」というシナリオで、`/api/report_templates/null`でAPIを叩いてしまうバグを修正

テンプレートの新規登録時は、コンポーネントにpropsとして渡されるテンプレートのidはnullになります。また現行の実装では、テンプレートの新規登録時に登録後のテンプレートのidをレスポンスとして受け取っていません。そのため、現行の実装において、「新規登録 -> ページを更新せず再度をモーダルを開き、再編集してテンプレート変更する」というシナリオで、テンプレート更新時に/api/report_templates/nullとしてAPIを叩いてしまっていました。

よって、新規登録時に、登録されたテンプレートのidをレスポンスとして受け取り、それをstateに保管する実装に修正する予定です。また、そのようなレスポンスを返せるよう、api/report_templates_controllerのcreateアクションに修正を加える予定です。

コードも含めた説明は下のPRのコメントにあります。 https://github.com/fjordllc/bootcamp/pull/7164/files#r1436196343

machida commented 9 months ago

@junohm410 共有ありがとうございますー🙏

junohm410 commented 8 months ago

@komagata CC: @machida お疲れ様です。本件、React化にあたり、もう一件既存のロジックによる問題点を解消できる点がありましたので、ご確認いただきたくご連絡しました。お手数をおかけしますが、よろしくお願いします。

React化に際して、処理で変更できる点

現状

編集中のテンプレートのマークダウンプレビューは、textareaMarkdowngemとMarkdownInitializer#renderによる2つの処理で行われている。

変更できる点

編集中のテンプレートのマークダウンプレビューは、textareaMarkdowngemによるプレビュー機能のみで記述可能。

確認いただきたい点

上の「変更できる点」で進めて問題ないか、ご判断いただきたいです。（これまでに相談した内容と同じく、今回は下記のようにすでに進めてしまっておりました🙇‍♂️別Issueとして取り組むべきであれば、元に戻すつもりです） https://github.com/fjordllc/bootcamp/pull/7164/files#r1436204117

変更できる理由の説明

現行のVueでは、編集中のテンプレートのマークダウンプレビューは、下記のようにtextareaMarkdowngemとMarkdownInitializer#renderによる2つの処理で記述されています。

// app/javascript/report_template_modal.vue
// 中略
    .a-markdown-input.js-markdown-parent
      .a-markdown-input__inner.js-tabs__content(
        v-bind:class='{ "is-active": isActive("template") }')
        textarea.a-text-input.a-markdown-input__textare.has-max-height(
          :id='`js-template-content`',
          :data-preview='`#js-template-preview`', // textareaMarkdownによるプレビュー実装
          v-model='editingTemplate',
          name='report_template[description]')
      .a-markdown-input__inner.js-tabs__content.a-long-text.is-md.a-markdown-input__preview(
        v-bind:class='{ "is-active": isActive("preview") }',
        v-html='markdownDescription') // MarkdownInitializer#renderによるプレビュー実装
        #js-template-preview // textareaMarkdownによるプレビュー実装

// 中略

    markdownDescription() {
      const markdownInitializer = new MarkdownInitializer()
      return markdownInitializer.render(this.editingTemplate) // MarkdownInitializer#renderによるプレビュー実装
    }

現状のVueにマークダウンプレビューの処理が2つある理由

↓こちらのコミットで、マークダウンプレビューがモーダルを開いたタイミングから反映されるようにするために、MarkdownInitializerによる処理が追加されたようでした。 https://github.com/fjordllc/bootcamp/pull/3419/commits/b060d85d4701e016e4053ae522d0e76d980ca35c

問題点

しかし、MarkdownInitializer#renderで得られたhtmlをv-htmlで当該divタグの子要素に入れた結果、同じくそのタグの子要素として記述されたdiv id="js-template-preview"がDOMに反映されず、textareaMarkdownの処理は無意味になっています。プレビューを表示する、という点では問題ありませんが、不必要な処理がそのままになっていることに違和感を感じます。

React化でできること

Reactでは最初の描画時にpropsのeditingTemplateをtextareaの初期値として渡し、そのあとにuseEffectでtextareaMarkdownでプレビュー表示の処理を行うことができるので、上記のコミットのようなケアは不要です。

（Vueの場合、textareaの初期値にpropsではなくデフォルト値の''が最初の描画時に使われ、その状態でtextareaMarkdownによるプレビュー処理が行われるため、モーダル起動時のプレビューには空文字が反映されてしまいます。その問題を解決するのが上記コミット。）

また、他の箇所のマークダウンの表示の実装を見てみると、

MarkdownInitializer#render -> すでにサブミットされたマークダウンによる投稿などの表示
textareaMarkdown -> テキストエリアに記入中のマークダウンのプレビュー表示

というように使い分けられているようだったので、React化にあたりtextareaMarkdown のみでプレビュー機能を実装し、MarkdownInitializerの方は削除する方針がいいのではと思っております。

komagata commented 8 months ago

@junohm410 回答の前に一点質問させてください。（この前提が違うとその後の議論が無意味になるため）

textareaMarkdown gemというのはnpmの間違いですかね？

junohm410 commented 8 months ago

@komagata はい、npmの間違いです。こちらは失礼いたしました🙇‍♂️

komagata commented 8 months ago

@junohm410 なるほどです〜。

:@komagata:でユーザーのアイコンが出るなど、FBC独自の機能も使えているのであればおっしゃっている方法で問題ないです〜。

junohm410 commented 8 months ago

@komagata ご回答ありがとうございました🙏

FBC独自の機能も使えているのであればおっしゃっている方法で問題ないです〜。

この方法でもFBCのMD独自機能は使えると判断しています。このtextareaを設定する際に、素のtextareaMarkdownを自分で使うのではなく、そのラッパーのTextareaInitializer.initializeというクラスメソッドを使っています。

// app/javascript/textarea-initializer.js

export default class {
  static initialize(selector) {
    const textareas = document.querySelectorAll(selector)
    if (!textareas.length) return

// 中略

    // markdown
    Array.from(textareas).forEach((textarea) => {
      /* eslint-disable no-new */
      new TextareaMarkdown(textarea, {
        endPoint: '/api/image.json',
        paramName: 'file',
        responseKey: 'url',
        csrfToken: CSRF.getToken(),
        placeholder: '%filenameをアップロード中...',
        afterPreview: () => {
          autosize.update(textarea)

          const event = new Event('input', {
            bubbles: true,
            cancelable: true
          })
          textarea.dispatchEvent(event)
        },
        plugins: [
          MarkdownItEmoji,
          MarkdownItMention,
          MarkdownItUserIcon,
          MarkdownItTaskLists,
          MarkDownItContainerMessage,
          MarkDownItContainerDetails,
          MarkDownItLinkAttributes,
          MarkDownItContainerSpeak
        ],
        markdownOptions: MarkdownOption
      })
      /* eslint-enable no-new */
    })

    // user-icon
    new UserIconRenderer().render(selector)

    // Convert selected text to markdown link on URL paste
    new TextareaMarkdownLinkify().linkify(selector)
  }

ここで独自機能用のプラグインやユーザーアイコンの設定を行なっているため、今回の日報テンプレートの入力・プレビューにおいてもFBC独自機能が使えます。実際に、Docs: フィヨルドブートキャンプで採用しているMarkdownの方言 | FBCにあるものが、この方法での開発環境で全て使えることも確認しました。

なので、いったんこの内容で進めさせていただければと思います。引き続き、何卒よろしくお願いいたします🙏

junohm410 commented 8 months ago

本番環境での動作確認ができましたので、本IssueはCloseさせていただきます。

fjordllc / bootcamp