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

パスワードリセットまわりの作成 #91

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

ogu-kazemiya
Copy link
Collaborator

close #63

遅れて申し訳ないです…!

Copy link
Collaborator

@mathsuky mathsuky left a comment

Choose a reason for hiding this comment

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

ありがとうございます!実装とても良さそうです。
いくつかhtml周りについてコメントしてあるのでご確認ください:pray:

src/views/ResetPasswordAfterMailView.vue Show resolved Hide resolved
入力されたメールアドレスがお使いのアカウントに登録されていない場合、メールは送信されません。
</p>
</div>
<div class="flex items-center justify-center gap-3 self-stretch">
Copy link
Collaborator

Choose a reason for hiding this comment

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

ここ意味のまとまりをわかりやすくするためにform要素とか使っても良さそう

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

「この画面を閉じる」ボタンは単なる画面遷移なので、まとめてform要素は適さないかなと思いました
入力欄とかがなくてもform要素にするものなんでしょうか?(よくわかっていないので教えてほしいです🙏)

Copy link
Collaborator

Choose a reason for hiding this comment

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

確かにサーバー側になんらかの情報を送るわけではないのでここでは使わなくて良さそうです,ごめんなさい:pray_shake:

<div class="flex flex-col items-start self-stretch p-2.5">
<div class="flex items-center justify-center gap-6 self-stretch">
<div class="flex w-50 items-center justify-end gap-2.5">
<span class="fontstyle-ui-body-strong text-[#3A3A3A]">パスワード</span>
Copy link
Collaborator

Choose a reason for hiding this comment

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

label要素とfor-idを使ってPasswordTextBoxと対応づけてください!
確認のところも同じようにお願いします。

/>
</div>
<div class="flex items-center justify-center gap-6 self-stretch pb-5">
<div class="h-4 w-50"></div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

変なプレースホルダーが入っているのが気になります。
おそらくパスワードについての注意書きをPasswordTextBoxとひとまとまりにすればflex + flex-colでいい感じに整列できると思うので,そうしてみてください。

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ひとまとまりにしましたが、flexだときれいに揃わなさそうな感じです
grid使えば、ラベル-テキストボックスの高さとテキストボックス-注意書きの横が同時にそろうかなと思うのでやってみます
grid使ったこと無くて今から調べる感じになりますが…

@ogu-kazemiya ogu-kazemiya requested a review from mathsuky January 5, 2025 12:41
Copy link
Collaborator

@mathsuky mathsuky left a comment

Choose a reason for hiding this comment

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

実装ありがとうございます!全体的にとても良いです。
余分なプレースホルダーの削除についてはおっしゃる通りflex+flex-colでは実現するのが大変かもしれないです(ラベルの方にpaddingを入れればまあいける?かもだけどそもそもそれは綺麗ではない)
いっていることが二転三転して申し訳ないですが,gridで実装していただけるとありがたいです!

入力されたメールアドレスがお使いのアカウントに登録されていない場合、メールは送信されません。
</p>
</div>
<div class="flex items-center justify-center gap-3 self-stretch">
Copy link
Collaborator

Choose a reason for hiding this comment

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

確かにサーバー側になんらかの情報を送るわけではないのでここでは使わなくて良さそうです,ごめんなさい:pray_shake:

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