Skip to content

Conversation

@kagomen
Copy link
Member

@kagomen kagomen commented Mar 19, 2024

概要

ABOUTセクションの実装

関連するIssue

closed #56

その他

  • スタイルのfont-extraboldが適用されなかった
    • そのためこのコミットではfont-boldとした
  • 試しにlayout.tsxのNotoSansJPのweightをコメントにして実行したら適用された
const notojp = Noto_Sans_JP({
  // weight: ["400", "500"],
  subsets: ["latin"],
  variable: "--font-notojp",
})
  • NotoSansJPはバリアブルフォントなのでweightの指定は不要らしい

    • バリアブルフォントとして使用する方がパフォーマンスも良くなるらしい?
    • 参考
  • 特に意図がなければweightの設定は不要かなと思ったのですが、ご意見あればお聞かせください!

残課題

  • layout.tsxでのweightの設定を変更する場合はイシューを立てる
  • リンク先未実装 →buttonタグをaタグに変更し、実装した
    • Next.jsのLinkは別リポジトリ(?)の場合は必要なかったような?
    • 調べます!! →next/linkはリロードせずにページを読み込む仕組みなので今回は不要

@kagomen
Copy link
Member Author

kagomen commented Mar 19, 2024

基本的なこと

  • ページ内のアクションを実行する場合はbutton
    • フォームの送信、画像の変更、ダイアログボックスの表示など
  • ページ外のリンクにアクセスする場合はa

参考

→プロジェクトのページに飛ばすのでaタグを使用する

Copy link
Collaborator

@pss-aileen pss-aileen left a comment

Choose a reason for hiding this comment

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

実装ありがとうございます!
2点ご提案です!
ご確認よろしくお願いいたします🙇‍♀️

src/app/page.tsx Outdated
<a
href="https://github.com/first-contributions-ja/first-contributions-ja.github.io"
className="bg-red-600 text-white text-lg px-4 py-3 rounded-xl hover:opacity-70 transition"
>● 詳しい手順はこちら</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

カンプに「⚫︎」を書いていたのですが
個人的にはあれは「仮アイコン」だったのと、
GitHubに飛ばしているので
GitHubアイコンが良いかな...と思いますがいかがでしょうか?

Copy link
Member Author

Choose a reason for hiding this comment

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

●も一応仮置きとしての記号だったのですが、現時点でロゴマークの制作できるかもわからないですし、GitHubのアイコンにするのが良さそうですね!
nextのアイコンの使い方等調べてから、実装します🚀🌟
ありがとうございます✨

src/app/page.tsx Outdated
<p>4. 変更をコミットする</p>
<p>5. コミット内容をプッシュする</p>
<p>6. プルリクエストを作成する</p>
<p>7. マージされるのを待つ</p>
Copy link
Collaborator

Choose a reason for hiding this comment

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

どちらでも良いかなと思ったのですが、
「順序リスト」として実装しても良いのかなと思いました!
olli ですね。

いかがでしょうか?

Copy link
Member Author

Choose a reason for hiding this comment

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

絶対その方が良いです……!
修正します!ありがとうございます✨

@pss-aileen
Copy link
Collaborator

  • 特に意図がなければweightの設定は不要かなと思ったのですが、ご意見あればお聞かせください!

こちらちょっと調査しますね!

Copy link
Member

@kazzyfrog kazzyfrog left a comment

Choose a reason for hiding this comment

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

PRありがとうございます!

簡単7ステップの内容に関して、以下のようにするのはどうでしょう?
最初のステップに、forkする事を追加する

例:1. Github上で[プロジェクト名]のリポジトリをフォークする

下記の3.4はまとめても良いかもですね!
そしたら、そのまま7ステップのままで良いかもです

3. ソースコードを変更する

4. 変更をコミットする

最後の7のマージは、個人的に以下のようにポップにしても良いのかなと思いました!

あなたの変更がメインプロジェクトに反映されます🎉

@kagomen
Copy link
Member Author

kagomen commented Mar 20, 2024

@kazzyfrog

ご提案ありがとうございます🙏

あなたの変更がメインプロジェクトに反映されます🎉

こちらすごく良さそうですね!
フォークの件も承知しました🌟

また、こちらの7ステップですが、雰囲気がわかりやすいよう仮置きとして記載したもので、READMEが完成した段階で再度編集する予定でした!
なので、もっと改善できるようであれば、README.mdが完成された段階などで、ご提案(もしくは勝手に編集していただいても大丈夫です!)いただけると幸いです!🌈

@pss-aileen
Copy link
Collaborator

const notojp = Noto_Sans_JP({
// weight: ["400", "500"],
subsets: ["latin"],
variable: "--font-notojp",
})

こちらの件、 #80 をみられたと思いますが
Variable fontsはそもそもNext.jsでweightを設定しなくて良いと書いてあったので、削る方向で良さそうです!!🙆‍♀️
https://nextjs.org/docs/pages/api-reference/components/font#weight

@kagomen
Copy link
Member Author

kagomen commented Mar 20, 2024

@pss-aileen
確認ありがとうございます!了解です!

調査中かと思いますが、こちらの件 #81 と合わせて変更できると良さそうです✨
イシュー立てておきます!

@kagomen kagomen mentioned this pull request Mar 20, 2024
2 tasks
@kazzyfrog
Copy link
Member

@kagomen

READMEが完成した段階で再度編集する予定でした!
なので、もっと改善できるようであれば、README.mdが完成された段階などで、ご提案(もしくは勝手に編集していただいても大丈夫です!)いただけると幸いです!🌈

確かにそうですね!了解です!🙌

@kagomen
Copy link
Member Author

kagomen commented Mar 20, 2024

#86 こちらで悩んで、勝手ながらMaterial-Iconsの導入をしてみました。
以下、変更後のビューになります。
スクリーンショット 2024-03-20 21 35 32

またライブラリの導入にあたって、package.jsonnext.config.jsに変更があるので、そちらの確認もお願いできればと思います。
next.config.jsの変更は、ライブラリのパフォーマンス改善を行うためのものです。
参考

@kazzyfrog
Copy link
Member

良いと思います!
コミットの分け方も見やすい👍

Copy link
Member

@kazzyfrog kazzyfrog left a comment

Choose a reason for hiding this comment

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

LGTM🥳

@kazzyfrog kazzyfrog merged commit 1b05fce into main Mar 20, 2024
@kazzyfrog kazzyfrog deleted the feat/#56_add_about_section branch March 20, 2024 15:14
@kazzyfrog kazzyfrog added the Type: ✨ feat 新機能の作成に関するもの label Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Type: ✨ feat 新機能の作成に関するもの

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ABOUTセクションの作成

4 participants