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.

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

microCMSの開発レビュー #30

Closed
tatsuya19881021 opened this issue Aug 31, 2022 · 4 comments
Closed

microCMSの開発レビュー #30

tatsuya19881021 opened this issue Aug 31, 2022 · 4 comments

Comments

@tatsuya19881021
Copy link

良いと思ったところ

  • src/utils/removeTags.tsにて、関数化しているところ。
  • Tooltip でリンクをホバーした際に表示しているところ。
  • その他色々あるかもしれませんが、レベル的に挙げきれず…

気になったところ

  • src/utils/formatDate.tsにて、自作フォーマット対応。

    • → day.js 使うと簡潔に書けそう。
    • convToTwoDigitString 関数は、padStart や slice のメソッドを使うとのもあり。
    • convToTwoDigitString 関数が2度出てくる。
  • getStaticProps関数のcatch処理。

    • returnのデータ構造は正しい?
    return {
      props: {
        err: 'データ取得で問題が発生しました。',
      },
    };

↓ 下記のような、data を入れなくても問題無いか?↓

    return {
      props: {
        data: {
          err: 'データ取得で問題が発生しました。',
        },
      },
    };
  • client.getとしている部分。

    • getList, getListDetailとすると TypeScript で扱いやすいとアーカイブのブログ作成動画で紹介されていたので、見直してみると良いかも。
  • portfolioDataなどの変数名。

    • → 受け売りですが、Data や Info などは意味が薄いので使わない方が良いという考え方があるようです。
    • portfolioData → portfolios とかにするなど。
    • (参考)変数名 data list 検索結果
  • src/pages/portfolio/[id].tsxでの、Layoutコンポーネントへ渡す値。

    • → データ取得失敗時の content が「Blog」となっている。
  • src/pages/contact.tsxにて、お問い合わせ完了時に通知を表示。

    • → APIから応答が返ってくるまでの間に、showNotificationloading: trueなどとしてあげて、
      返ってきたら、その旨を通知するのも良いかな〜と思いました。
  • HomeページのPortfolio。

    • → 画像と説明の幅が揃っていないので、figmaと似た感じで合わせるとキレイそうと思いました。
  • Contactページの各入力エリアの幅が狭め。

    • → figmaのデザインに合わせて、PC表示時はもう少し広くても良いと思いました。
@tatsuya19881021
Copy link
Author

色々書かせて頂きました。

自分の勉強不足などで不適切な内容があれば、すみません。

途中まで直に書いていましたが、一度ブラウザ戻って入力内容を飛ばしてしまったので、
ちょっと気持ちが…ってなり、箇条書きのような書き方とか混ざっているのはご了承下さい。

一応、下記のやつは別出ししました。(ここで伝える形で良いのやら…?)

  • src/components/layout/Layout.tsxの引数名
    • → Typo。

@tatsuya19881021
Copy link
Author

tatsuya19881021 commented Aug 31, 2022

直接関係ないところですが…ザーッと見た際に気付いたので、参考として書かせて頂きます。

  • アイコンをSVGで用意されているようですが、React Iconsなどを用いると自前のファイルは必要なくなるかな〜と思いました。

@pitang1965
Copy link
Owner

沢山の指摘ありがとうございます。どれもごもっともです。まだ終えていないやつは、この後やっていきます。

以下を除き、issueを別に切り出したので、こちらはクローズとさせていただきます。

  • src/components/layout/Layout.tsxのtypo → 修正しました
  • アイコンについて → 今回、Figmaと同じデザインにしたいと思ったのでこのようにしました。このプロジェクトでは、Mantineが使っているアイコンライブラリも使用していますが、そちらのTwitterアイコンなどは、デザインが簡略化したものとなっていました。
    bitmoji

@tatsuya19881021
Copy link
Author

沢山の指摘ありがとうございます。どれもごもっともです。まだ終えていないやつは、この後やっていきます。

以下を除き、issueを別に切り出したので、こちらはクローズとさせていただきます。

  • src/components/layout/Layout.tsxのtypo → 修正しました
  • アイコンについて → 今回、Figmaと同じデザインにしたいと思ったのでこのようにしました。このプロジェクトでは、Mantineが使っているアイコンライブラリも使用していますが、そちらのTwitterアイコンなどは、デザインが簡略化したものとなっていました。

ちょっと気になる作り込みなど、あれば別途Slackなりで確認して頂けると幸いです。

正直、SSGとかそれ関連のコードはまだ理解が足りておらず、ニュアンスとしては分かるものの、
アドバイスやら意見やらを言えるほどではないので🙏

ふむふむ。確かにtabler iconsには良い感じのなかったですよね〜
自分はFont Awesome?のやつを採用しておりますが、React Iconsを利用することで
tabler iconsと他のアイコンなども使えて重宝しております。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants