-
Notifications
You must be signed in to change notification settings - Fork 83
feat: サーバが出した画像エラーを目立たせるようにする #522
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
Conversation
2048fa9
to
781d67a
Compare
@0Delta PRを作成いただきありがとうございます! お手数ですが、以下の手順でローカルサーバを起動し、今回の変更がわかるようにスクリーンショットを撮影して共有いただくことは可能でしょうか? 🙏 共通のセットアップ# プロジェクトルートで実行
pnpm i
pnpm run build zenn-cliのセットアップcd packages/zenn-cli
cp .env.example .env
pnpm i
# ローカルサーバが起動、ブラウザから動作確認が可能
pnpm run dev |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ありがとうございます!
チームで話し合いまして、本文中の画像エラーを画面上部にも表示するというアイデアは受け入れるつもりです。
コードについていくつかコメントしましたので、対応いただけますと幸いです。
`zenn-cli/server` で出力するタグに専用の要素を追加し、それを `zenn-model` からcheerioを利用して検出。 該当があるなら、既存のエラーと同等に出してあげることで UX を改善する。
781d67a
to
681b68a
Compare
@cm-dyoshikawa 諸々修正したコミット作りました! |
@0Delta マージしました!対応ありがとうございました! 当面はzenn-cliのcanaryバージョンでお使いいただけます。 |
画像の参照エラー等を、既存のエラーと同じ場所に出してあげることで UX を改善する。
内部的には
zenn-cli/server
で出力するタグに専用のクラスを追加し、それをzenn-model
から全文検索で検出している。将来的にクラスを増やすことで別メッセージを出力させるようにすることも可能。
2つのパッケージに同じ単語を入れなければならない暗黙的なルールができてしまう所は要検討か。
📑 Summary
プルリクエストに含む内容の簡潔な記述
Resolves zenn-dev/zenn-community#681
📋 Tasks
プルリクエストを作成いただく際、お手数ですが以下の内容についてご確認をお願いします。
canary
ブランチに対するプルリクエストであるより詳しい内容は Pull Request Policy を参照してください。