Conversation
|
""" Walkthrough「speak」コンテナプラグインにカスタムマーカー「:」とバリデーション関数が追加され、スピーカー名やアバターURLのパースとレンダリングが拡張されました。ユーザーリンクの有無に応じた表示も可能になっています。加えて、スピーカー表示のCSSスタイルがflexレイアウト対応で調整されました。さらに、システムテストに新たな「speak」ブロックのレンダリング検証が追加されました。加えて、DBスキーマの複数テーブルで文字列長制限の削除やカラム追加・削除、インデックス名変更などの変更が含まれています。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~40 minutes Possibly related PRs
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 RuboCop (1.76.1)test/system/markdown_test.rbrubocop-minitest extension supports plugin, specify Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
app/javascript/markdown-it-container-speak.js (1)
38-58: コード重複の削除を検討3つの異なる分岐で類似のHTMLテンプレートが生成されています。保守性向上のため、共通のヘルパー関数を作成することを検討してください。
以下のような共通関数を作成できます：
function createSpeakHTML(speakerName, avatarSrc, userLink = null, title = null) { const linkAttrs = userLink ? `href="${userLink}"` : '' const imgAttrs = title ? `title="${title}"` : `title="${speakerName}"` const imgClass = userLink ? 'js-user-icon a-user-emoji' : 'a-user-emoji speak__speaker-avatar' const dataUser = userLink ? `data-user="${speakerName}"` : '' return `<div class="speak"> <div class="speak__speaker"> <a ${linkAttrs} class="a-user-emoji-link"> <img src="${avatarSrc}" alt="${speakerName}" ${imgAttrs} class="${imgClass}" ${dataUser}> <span class="speak__speaker-name">${speakerName}</span> </a> </div> <div class="speak__body">` }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/javascript/markdown-it-container-speak.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.js
Instructions used from:
Sources:
⚙️ CodeRabbit Configuration File
🧠 Learnings (1)
📓 Common learnings
Learnt from: tyrrell-IH
PR: fjordllc/bootcamp#8807
File: app/views/welcome/job_support.html.slim:391-392
Timestamp: 2025-06-21T22:30:20.116Z
Learning: このプロジェクトにはRailsの組み込みmarkdownヘルパが利用できないため、markdownのHTML変換には自前でヘルパを実装する必要がある。
🧬 Code Graph Analysis (1)
app/javascript/markdown-it-container-speak.js (1)
app/javascript/escapeHtml.js (1)
escapeHTML(1-8)
🔇 Additional comments (1)
app/javascript/markdown-it-container-speak.js (1)
61-61: 終了タグの処理を確認終了タグの処理が正しく実装されていることを確認しました。
|
@su-su-su-su |
|
@machida |
|
@su-su-su-su すいません、遅くなりました。今 push しました。 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/javascript/stylesheets/config/mixins/_long-text-style.sass(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build_and_test
su-su-su-su
force-pushed
the
feature/add-arguments-to-speak-block
branch
from
8bfb883 to
0c0ab2b
Compare
|
@riq0h |
|
@su-su-su-su お疲れ様です！ 個人的にはお受けするのはまったく構わないのですが、僕はすでに修了している身なので必要以上に依頼を受けてしまうと他の方の学習機会を損ねてしまわないか気がかりです🙇 一旦、現役生の中で手が空いている方を探していただいてもよろしいでしょうか🙏 |
|
@riq0h |
|
@e-yanagita-gs |
|
@su-su-su-su |
|
@e-yanagita-gs |
|
@tyrrell-IH |
|
@su-su-su-su |
|
返信ありがとうございます！ |
|
@sjabcdefin |
|
@su-su-su-su |
|
@sjabcdefin |
|
@smallmonkeykey |
|
@su-su-su-su |
|
@smallmonkeykey |
smallmonkeykey
left a comment
smallmonkeykey
left a comment
There was a problem hiding this comment.
@su-su-su-su
全体としては意図が伝わりやすく、読みやすいコードだと思いました！
2点コメントしましたので、ご確認お願いします🙏
There was a problem hiding this comment.
if の条件分岐が少し多めに見えました👀
ネストも少し深くなっているので、どのパターンでどういう処理が走るのかを把握するのに少し時間がかかるかも？と感じました。
There was a problem hiding this comment.
ご指摘ありがとうございます！
確かに条件分岐が多く、ネストも深くて理解しにくい状態でした。
早期リターンパターンでリファクタリングし、処理パターンが分かりやすいように修正いたしました。
| const speakerName = escapeHTML(parenMatch[1].trim()) | ||
| const avatarUrl = escapeHTML(parenMatch[2].trim()) | ||
|
|
||
| if (tokens[idx].nesting === 1) { |
There was a problem hiding this comment.
tokens[idx].nesting === 1 のような条件は、一見すると何を判定しているのかが少し分かりづらいかもしれません💦
たとえば、以下のように関数として切り出しておくと、より意図が伝わりやすいかなと思います。
function isOpeningTag(token) {
return token.nesting === 1;
}
こうすることで、他の方が markdown-it のソースコードを毎回見に行かなくても、ある程度意図が伝わるようになってレビューや保守がしやすくなるんじゃないかな〜と思います！
ただ、他のファイルでも tokens[idx].nesting === 1 のまま使われているようだったので、今回もそのままの記述で統一しておくのもありだと思います 🙆♀️
もし関数化して明示した方が良さそうであれば、おまかせします〜！
There was a problem hiding this comment.
ご提案ありがとうございます！
なるほどですね！勉強になります🙏
今回は他のmarkdown-it関連ファイルとの統一性を優先し、既存のコードスタイルに合わせました。
このファイルだけ特別にすると混乱を招く可能性があると判断しました🙇♂️
su-su-su-su
force-pushed
the
feature/add-arguments-to-speak-block
branch
from
5adfbd6 to
e994314
Compare
|
@smallmonkeykey |
smallmonkeykey
left a comment
smallmonkeykey
left a comment
There was a problem hiding this comment.
@su-su-su-su
Approveさせていただきます〜！
正規表現、とても勉強になりました🙏
|
@smallmonkeykey |
|
@komagata お手すきの際にご確認お願いいたします。 |
8 participants
Issue
概要
Markdown内の独自記法のspeakブロック機能を追加しました。
引数として
@ユーザー名、(名前, 画像URL)、名前のみの3パターンに対応するようにしています。それぞれに適したアバターと名前を表示。
(名前, 画像URL)の画像はランダムです。変更確認方法
feature/add-arguments-to-speak-blockをローカルに取り込むforeman start -f Procfile.devでサーバーを立ち上げます。komagataでログインScreenshot
変更前
プレビュー画面
公開後画面
変更後
プレビュー画面
公開後画面
Summary by CodeRabbit
Summary by CodeRabbit