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

textarea に画像をアップロードしたときの記法を マークダウン形式から素のHTML形式へ変更 #7448

Merged
merged 5 commits into from
Apr 15, 2024
Merged

textarea に画像をアップロードしたときの記法を マークダウン形式から素のHTML形式へ変更 #7448

merged 5 commits into from
Apr 15, 2024

Conversation

goruchanchan
Copy link
Contributor

@goruchanchan goruchanchan commented Feb 27, 2024 •
edited
Loading

Issue

概要

textarea に画像をアップロードした際の記法が、マークダウン形式から HTML 形式になりました。

変更確認方法

  1. feature/textarea_image_format_markdown_to_htmltag をローカルに取り込む
  2. foreman start -f Procfile.dev でアプリを立ち上げる
  3. komagata でログイン
  4. お知らせ作成のフォームページにアクセスし、テキストエリアに画像をアップロードし、テキストが <img src="ファイルパス" width="100" height="100" loading="lazy" decoding="async" alt=”image.png”> となっていることを確認する
    ※ textarea への画像アップロードに対応するページは複数ありますが、代表例として『お知らせ作成』のみを確認いただこうと思います。

Screenshot

変更前

  • ファイル名がある場合

    image

  • ファイル名がない場合

    image

変更後

  • ファイル名がある場合

    image

  • ファイル名がない場合

    image

@goruchanchan goruchanchan changed the title textarea に画像をアップロードしたときni、画像は Markdown 記法はではなく素のHTMLタグ textarea に画像をアップロードしたときの記法を マークダウン形式から素のHTML形式へ変更 Feb 27, 2024
@goruchanchan goruchanchan force-pushed the feature/textarea_image_format_markdown_to_htmltag branch from 90d1d0f to 082d2a9 Compare February 28, 2024 14:51
@goruchanchan goruchanchan self-assigned this Feb 28, 2024
@goruchanchan goruchanchan marked this pull request as ready for review February 29, 2024 14:42
@goruchanchan
Copy link
Contributor Author

@kurumadaisuke お疲れ様です！お手数おかけして申し訳ないのですが、レビューいただけないでしょうか？お忙しいようであれば遠慮なく仰って頂ければと思います🙇‍♂️

@kurumadaisuke
Copy link
Contributor

@goruchanchan
お疲れ様です！
こちら対応可能です！！
レビュー期限があればご教示お願いします🙏

@goruchanchan
Copy link
Contributor Author

@kurumadaisuke
ありがとうございます🙇‍♂️期限等は特にありませんのでお手隙の時にお願いします🙇‍♂️

kurumadaisuke
kurumadaisuke reviewed Mar 4, 2024
View reviewed changes
Copy link
Contributor

@kurumadaisuke kurumadaisuke left a comment

Choose a reason for hiding this comment

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

@goruchanchan
遅くなって申し訳ありませんmm
こちら動作確認してみると、なぜか画像が貼られていない状態で作成されているのですが確認手順って以下の通りですよね😰？

feature/textarea_image_format_markdown_to_htmltag をローカルに取り込む
foreman start -f Procfile.dev でアプリを立ち上げる
komagata でログイン
[お知らせ作成](http://localhost:3000/announcements/new)のフォームページにアクセスし、テキストエリアに画像をアップロードし、テキストが <img src="ファイルパス" width="100" height="100" loading="lazy" decoding="async" alt=”image.png”> となっていることを確認する
※ textarea への画像アップロードに対応するページは複数ありますが、代表例として『お知らせ作成』のみを確認いただこうと思います。
スクリーンショット 2024-03-04 22 30 38

@goruchanchan
Copy link
Contributor Author

@kurumadaisuke
ご確認ありがとうございます！！ファイル名に空白があると alt 属性が正しく設定されてない不具合がありましたので、修正しました🙇‍♂️お手数をおかけしますが、再度ご確認よろしくお願いいたします🙇‍♂️

@kurumadaisuke
Copy link
Contributor

@goruchanchan
ご対応ありがとうございます🙇‍♂️
画像として表示されるようになったことを確認しました〜！！

セキュリティ

issueの範囲外かもですが、セキュリティってどうなんだろう〜っと思って
textareaに以下を組み込んだら「送信」ボタン作成できて、送信先自由に指定できるみたいです
issueの範囲外だと思うので一応komagataさんに確認していただければと🙏

<form action="https://example.com/submit" method="post">
<input type="hidden" name="csrf_token" value="1234567890">
<input type="submit" value="送信">
</form>
スクリーンショット 2024-03-05 21 31 29

gifの場合

gifの場合は今回は対象外ですかね？？
試しに貼ってみると <img src="ファイルパス" width="100" height="100" loading="lazy" decoding="async" alt=”image.png”> の形式になっていなかったので共有しておきます🙏
スクリーンショット 2024-03-05 21 39 33

@goruchanchan
Copy link
Contributor Author

@kurumadaisuke
確認ありがとうございます🙇‍♂️

セキュリティ

確認しておこうと思います！ちなみに FBC アプリのプロダクト環境でもできる気がします🤔
（左側がプロダクト環境、右側が本ブランチ）

22acb8771d9c10ab237a2c11e7a4132d

gif

gif ですが下記のように プロダクト環境と同様にアップロードできている認識ですが、どのようにアップロードしたのでしょうか？下記は gif をコピーして、テキストエリアへ貼り付けてアップロードしています。（左側がプロダクト環境、右側が本ブランチ）

Image from Gyazo

@goruchanchan
Copy link
Contributor Author

@komagata
お疲れ様です！くるまさんからのこちらの指摘についてですが、テキストエリアのプレビューにはセキュリティ対応が必要でしょうか？

@kurumadaisuke
Copy link
Contributor

@goruchanchan
https://gyazo.com/から「シェア」⇨gifコピーでやっていました！！
自分の方法が変だったのかもしれません🙏
大丈夫だったので大丈夫だと思います🙏

kurumadaisuke
kurumadaisuke approved these changes Mar 6, 2024
View reviewed changes
Copy link
Contributor

@kurumadaisuke kurumadaisuke left a comment

Choose a reason for hiding this comment

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

@goruchanchan
一旦、駒形さんへの確認中のものは置いておいて
私からはコード確認しましたのでApproveします🙏

@goruchanchan
Copy link
Contributor Author

goruchanchan commented Mar 6, 2024 •
edited
Loading

@kurumadaisuke

[https://gyazo.com/から「シェア」⇨gifコピーでやっていました！！]

なるほどです！おそらくですが、gif の形式が異なっていたのは、下図のリンクコピー結果を貼っていたからじゃないかと思っています🙇‍♂️
Image from Gyazo

リンクコピーの場合、テキストエリアで FBC アプリへのアップロードしているのではなく、gyaza へのアップロード結果へのリンクを貼っていることになったんじゃないかなと思います🙇‍♂️
一旦ダウンロードした上で、FBC アプリへテキストエリア経由でアップロードしていただければ、挙動確認いただけるかと思います。
Image from Gyazo

@goruchanchan
Copy link
Contributor Author

@kurumadaisuke
お忙しい中、レビューいただきありがとうございました🙇‍♂️静止画像しか頭になかったですが、gif は盲点でした🙇‍♂️テストすべき対象の範囲の観点、勉強になりました🙇‍♂️
また、セキュリティの件についてですが、本日のふりかえりミーティングにて別Issueで対応予定とのことでした🙇‍♂️

１点、申し訳ないですが、誤って再度レビューアー登録してしまいましたので、お手数ですが再度、ご確認いただきたく思います🙇‍♂️

@komagata
メンバーの承認いただけましたので、お手すきの時にご確認よろしくお願いします🙇‍♂️

kurumadaisuke
kurumadaisuke approved these changes Mar 7, 2024
View reviewed changes
Copy link
Contributor

@kurumadaisuke kurumadaisuke left a comment

Choose a reason for hiding this comment

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

再度、Approveします〜🙆‍♂️

@komagata
Copy link
Member

komagata commented Mar 9, 2024

@goruchanchan 今頃で大変申し訳ないです。

https://github.com/komagata/textarea-markdown/blob/main/src/textarea-markdown.js#L16

上記のように外から、下記のような画像の置換のためのフォーマット文字列（もしくはそれに用いる関数）を設定できるようになれば、最小限の上書きで済み、かつカスタマイズ可能になりそうに思いました。

https://github.com/komagata/textarea-markdown/blob/main/src/textarea-markdown.js#L163

"ビデオの時の処理"、"画像の時の処理"、"その他"みたいな感じで設定できるといいかなと思います。

今更思いついて済みません🙇‍♂️

goruchanchan
goruchanchan commented Mar 10, 2024
View reviewed changes
app/javascript/textarea-markdown-to-html.js Outdated
/%filename/,
file.name
)} >`
const text =
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ここは、動画、他ファイル形式があることを失念していたので、継承元のコードに戻した方が自然と判断し戻しました🙇‍♂️

@goruchanchan
Copy link
Contributor Author

goruchanchan commented Mar 10, 2024 •
edited
Loading

@komagata
ご確認ありがとうございます！また、アドバイスありがとうございます！フォーマット文字列を使う方法で対応してみましたので、お手数をおかけしますが再度ご確認お願いします🙇‍♂️

また、動画、その他形式も合わせて対応しました。

image

@goruchanchan goruchanchan force-pushed the feature/textarea_image_format_markdown_to_htmltag branch from 54a5cf0 to e417e84 Compare March 10, 2024 14:01
@komagata
Copy link
Member

@goruchanchan ちょっと話が伝わってないかもしれません。

#7448 (comment)

こちらでお話しした内容は、bootcamp側で上書きする内容を最小化するために、textarea-markdownのnpm自体を外部からフォーマット文字列を設定できるように修正するというお話です。

bootcamp側ではその新しいバージョンのnpmを使って、フォーマット文字列をnpm外部から設定してあげる形になると思います。

@goruchanchan
Copy link
Contributor Author

@komagata

bootcamp側ではその新しいバージョンのnpmを使って、フォーマット文字列をnpm外部から設定してあげる形になると思います。

失礼しました🙇‍♂️textarea-markdownに対してPRを出して、更新された npm をbootcapm側で使うということですね🙇‍♂️そのように対応いたします🙇‍♂️

@goruchanchan
Copy link
Contributor Author

@komagata
お疲れ様です！お手すきの時に textarea-markdown にマージいただいた内容を textarea-markdown npm へ反映お願いいたします🙇‍♂️

@komagata
Copy link
Member

@goruchanchan いただいたPRを取り込んだ textarea-markdown 1.5.1をリリースしました～

@goruchanchan
Copy link
Contributor Author

@komagata ありがとうございます！！

@goruchanchan goruchanchan force-pushed the feature/textarea_image_format_markdown_to_htmltag branch from 0f99963 to f23fff2 Compare March 30, 2024 13:50
@goruchanchan
Copy link
Contributor Author

@komagata 更新いただいた npm を導入いたしました🙇‍♂️お手すきの時にご確認お願いいたします🙇‍♂️

komagata
komagata approved these changes Apr 15, 2024
View reviewed changes
Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

コードがすごいすっきりしましたね！
ありがとうございます～！

@komagata komagata merged commit ebbebe4 into main Apr 15, 2024
2 checks passed
@komagata komagata deleted the feature/textarea_image_format_markdown_to_htmltag branch April 15, 2024 23:23
@github-actions github-actions bot mentioned this pull request Apr 15, 2024
Merged
14 tasks
@goruchanchan goruchanchan mentioned this pull request Aug 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kurumadaisuke kurumadaisuke kurumadaisuke approved these changes

@komagata komagata komagata approved these changes

Assignees

@goruchanchan goruchanchan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@goruchanchan @kurumadaisuke @komagata