Skip to content

Conversation

@kazuhitogo
Copy link
Collaborator

Description of Changes

Add support for customizing the logo and title displayed on the landing page through a configuration file.

Checklist

  • Modified relevant documentation
  • Verified operation in local environment
  • Executed npm run cdk:test and if there are snapshot differences, execute npm run cdk:test:update-snapshot to update snapshots

Related Issues

#1338

<img src={customLogoUrl} alt={title || 'Logo'} className="mr-5 size-20" />
) : (
<AwsIcon className="mr-5 size-20" />
);
Copy link
Contributor

Choose a reason for hiding this comment

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

もう少しシンプルかつメモ化して実装したほうが良さそうな気がしました。
customLogoUrl を state で管理する必要がないと思うので、以下のような感じ。

const displayLogo = useMemo(() => {
  // logoPath があったら <img/> を返し、なければ <AwsIcon/> を返す
}, [logoPath]);

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

修正します

const logoPath = import.meta.env.VITE_APP_BRANDING_LOGO_PATH;
const title = import.meta.env.VITE_APP_BRANDING_TITLE;

console.log('Branding config:', { logoPath, title });
Copy link
Contributor

Choose a reason for hiding this comment

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

消し忘れ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

おっと、、、消し忘れです

const { enabled } = useUseCases();
const { setIsShow, init } = useInterUseCases();
const { t } = useTranslation();
const { logoPath, title } = useBranding();
Copy link
Contributor

Choose a reason for hiding this comment

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

これビルド時に組み込まれている環境変数から読み込んでいるだけの定数なので、hook 化するか微妙ですね。ragEnabled などと同様に、上部で定数として読み込んでも良い気がしました。なにか今後の展望的に hook 化したほうが良さそう的なことありますか?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ありがとうございます。
無理やり理由つけるなら、将来的な話だと動的に変えたい、ってくらいですが、今そんな話は特にないので、環境変数を直接読むようにします。

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

Successfully merging this pull request may close these issues.

2 participants