[TypeScript演習]Google拡張機能の課題追加 #80
Conversation
aAnnaChiba
changed the title
|
スペルミスだと思いますが、 以下を確認ください。
|
|
|
||
| // TODO 削除ボタンの作成 | ||
|
|
||
| bookmarkList.appendChild(title) |
There was a problem hiding this comment.
複数の要素を描画するときに1つ1つ DOM に追加していくのはパフォーマンスに悪いので DocumentFragment を使用するようにしましょうか。
https://zenn.dev/toshihide2000/scraps/87957b8360cf84
There was a problem hiding this comment.
ありがとうございます 🙇
165516b で修正しましたが、このようなかたちであっていますか 🤔
There was a problem hiding this comment.
空の fragment が繰り返し append されるだけで無意味なコードになっちゃってます。
forEach の中で毎回画面が再描画されるのを避けようという話で、そのために
- 描画されていない(=documentのどこにも存在しない) fragment を作る(
createDocumentFragment()) forEachの中ではその fragment に append することで再描画を防ぐ- 中身がすべて出来上がった後で、描画されている要素(
bookmarkList) に fragment ごと append
とすることで1回の再描画で済まそう、ということです。
const fragment = document.createDocumentFragment();
// TODO forEachを使ってブックマークリストの要素を表示してみよう
bookmarks.forEach((bookmark) => {
// ...
fragment.appendChild(title);
});
bookmarkList.appendChild(fragment);There was a problem hiding this comment.
すいません、 QuenstionProposal.md の方だけ見てコメントしましたが、answer.ts の方は合ってますね。
There was a problem hiding this comment.
關さんのコメントと被りますが、 QuestionPropsal.md は forEach の中で bookmarkList.appendChild(fragment); が実行されているのでそこだけ修正ください。
answer.ts は OK です。
| @@ -0,0 +1,130 @@ | |||
| // // 問題 | |||
There was a problem hiding this comment.
わざわざ全部コメントにしなくても良いんじゃないでしょうか。
|
|
||
| *.log | ||
|
|
||
| .DS_Store |
There was a problem hiding this comment.
(nit/optional) 細かいですが、テキストファイルの末尾には改行があるべきです。参考
| min-width: 300px; | ||
| } | ||
| form { | ||
| margin-bottom: 10px; |
There was a problem hiding this comment.
(nit/optional) HTML/CSS研修でどう教えているかにも依りますが、margin-block-end 等の logical property を使うことが一般的には良いです。
There was a problem hiding this comment.
HTML/CSS 研修で論理プロパティについて触れていただいたので、ぜひ使っていただけると良いです。
| <!DOCTYPE html> | ||
| <html lang="ja"> | ||
| <head> | ||
| <meta charset="UTF-8" /> |
There was a problem hiding this comment.
(nit/optional) JSXやXHTMLではないので、void element の / は不要です。
| </form> | ||
| <ul id="bookmark-list"></ul> | ||
|
|
||
| <script src="dist/app.js"></script> |
There was a problem hiding this comment.
(nit) 誤差レベルですが、 <head> タグの中で defer つけて書いたほうが読み込みが少し早くなります。
|
|
||
| - [Q1. 型定義をする](#q1-型定義をする) | ||
| - [Q2. インターフェースの継承を使ってみる](#q2-インターフェースの継承を使ってみる) | ||
| - [Q3. オプショナルチェーンを使って、リファクタリングしてみる](#q3-オプショナルチェーンを使ってリファクタリングしてみる) |
| // TODO forEachを使ってブックマークリストの要素を表示してみよう | ||
| bookmarks.forEach((bookmark) => { | ||
| // タイトルとURLのリンクを作成 | ||
| const title = document.createElement('li'); |
There was a problem hiding this comment.
(want) このコメントと変数名だと、このタグにタイトルが入っていそうですが、実際はそうではないので違和感があります。item, listItem, li とかにリネームしたいです。
| // TODO: Q7. 新しいブックマークの作成をしてみる | ||
| const newBookmark: Bookmark = { | ||
| id: crypto.randomUUID(), | ||
| title, | ||
| url, | ||
| category: category || undefined, | ||
| createdAt: new Date(), | ||
| }; |
There was a problem hiding this comment.
(optional) 他の関数の分け方を見ると、ここも createBookmark(title, url, category) みたいに関数に分けたほうが自然に感じます。
|
|
||
| // 初期化 | ||
| const bookmarks = loadBookmarks(); | ||
| document.addEventListener('DOMContentLoaded', () => renderBookmarks(bookmarks)); |
There was a problem hiding this comment.
(want) 既に bookmarkList を取得した後なので、DOMContentLoaded イベントを待たずにここで即 renderBookmarks(bookmarks); と呼び出しても変わらないと思います。
defer にしとけばスクリプトの実行がDOMの構築後になりますし、これで特に問題ないはずです。
| return { | ||
| root: "src", | ||
| build: { | ||
| outDir: "../dist", |
There was a problem hiding this comment.
(ask) tsconfig.json の outDir と階層がズレてるように見えますが、どういうことでしょう？
| export default defineConfig((opt) => { | ||
| return { |
There was a problem hiding this comment.
(want) opt が使われていないようです。
(optional/nit) return を省略した書き方にできます。ネストが1段減らせますが、前処理等が増える可能性を考えているならこのままでも良いです。
内容
TypeScript 演習で、時間が余ったとき用の自由課題として、Google拡張機能の課題を追加しました。
問題案はQuestionProposal.mdに、答えのtsファイルはanswer.tsにあります。
問題の追加や、リファクタなど、指摘がありましたらレビューお願いします。
TODO