Feature: custom OIDC provider #213
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wadabee
reviewed
Apr 5, 2024
| } from 'react'; | ||
| import { BaseProps } from '../@types/common'; | ||
| import { Auth } from 'aws-amplify'; | ||
| import { Button, Text, Loader } from '@aws-amplify/ui-react'; |
There was a problem hiding this comment.
AmplifyUIはAuthenticator以外利用していないので、他の画面に合わせて利用しないように修正をお願いします。
（ログイン画面なのであえてAmplifyUIを使ったのかもしれませんが、できればメインの画面に合わせたいです）
Loaderは以下のソースを再利用できる形に変えると良さそうです！
There was a problem hiding this comment.
Amplify UI使わないよう変更しました！
| return ( | ||
| <> | ||
| {loading ? ( | ||
| <div className="grid grid-cols-1 justify-items-center gap-4"> |
There was a problem hiding this comment.
1項目しかないので、gridではなくflexで良いと思います。
| <Button | ||
| variation="primary" | ||
| onClick={() => signIn()} | ||
| className="mt-6 w-60"> |
There was a problem hiding this comment.
超細かくて恐縮ですが、言語によって幅の広さが変わるので、ボタンのwidth指定はあまり好ましくないです。
見栄えの観点でwidth指定していると思いますが、言語によって余白幅が変わり見栄えが変わってしまうという観点です（今回は大丈夫と思いますが、収まり切らずに文字がオーバーフローするリスクもあるので）
px-[number] で余白で調整した方が、ベターです！
もしログインボタンを強調したいのであれば、枠線をつけてその領域に注目がいくようにすれば良いかなと思います。
There was a problem hiding this comment.
なるほど、その観点はなかったですね。勉強になります！pxで調整しました。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.