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

fix: enhance oauth/authorization UI #39

Merged
merged 1 commit into from
Nov 10, 2024
Merged

fix: enhance oauth/authorization UI #39

merged 1 commit into from
Nov 10, 2024

Conversation

sor4chi
Copy link
Member

@sor4chi sor4chi commented Nov 9, 2024

No description provided.

@sor4chi sor4chi self-assigned this Nov 9, 2024
@sor4chi sor4chi requested a review from a01sa01to November 9, 2024 04:09
Copy link

cloudflare-workers-and-pages bot commented Nov 9, 2024

Deploying saitamau-maximum-auth with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4b3e776
Status: ✅  Deploy successful!
Preview URL: https://3a3121cf.saitamau-maximum-auth.pages.dev
Branch Preview URL: https://feat-oauth-ui.saitamau-maximum-auth.pages.dev

View logs

@sor4chi sor4chi force-pushed the feat/oauth-ui branch 2 times, most recently from 8e82984 to 33aac0c Compare November 9, 2024 08:08
Copy link
Member

@a01sa01to a01sa01to left a comment

Choose a reason for hiding this comment

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

とりあえずよさそう (ここまでの量の html が api 内部にあるの個人的にはちょっと気持ち悪いけど)
結局ユーザー情報表示させる?

@sor4chi
Copy link
Member Author

sor4chi commented Nov 9, 2024

任せる
これはフォルダ設計考えないとね

Copy link
Member

@a01sa01to a01sa01to left a comment

Choose a reason for hiding this comment

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

  • ユーザー情報あると今ちゃんとログインできているのか確認できてうれしいので追加したい
  • OAuth Client にも logo_url があるのでそれもあるとよさそう (まあなければ適当に...)
  • 個人的にはこの量あるなら Remix 下に置いちゃいたいかも (もしいい方法があれば...)

@sor4chi
Copy link
Member Author

sor4chi commented Nov 10, 2024

ユーザー情報あると今ちゃんとログインできているのか確認できてうれしいので追加したい
OAuth Client にも logo_url があるのでそれもあるとよさそう

次のPRで良い?今回は単に既存SSRのリファクタなので

個人的にはこの量あるなら Remix 下に置いちゃいたいかも

これはそもそもoauth機能とaccount機能を一緒に置いてるのが悪くて、基本的に別システムなのでパッケージ or サービス分けるくらいにしたほうが良さそう
そもそもoauth serverがremixに介入するのはキモくて、remixからDBは触りたくない

@a01sa01to
Copy link
Member

これはそもそもoauth機能とaccount機能を一緒に置いてるのが悪くて、基本的に別システムなのでパッケージ or サービス分けるくらいにしたほうが良さそう

これは作りながら思ったので分けましょうかね...

@sor4chi sor4chi merged commit dbea94d into main Nov 10, 2024
7 checks passed
@sor4chi sor4chi deleted the feat/oauth-ui branch November 10, 2024 05:01
@a01sa01to a01sa01to mentioned this pull request Nov 11, 2024
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