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

feat: FormControlのエラー表示設定方法を変更し、より確実に設定されるように修正する #4925

Merged
merged 14 commits into from
Sep 26, 2024

Conversation

AtsushiM
Copy link
Member

@AtsushiM AtsushiM commented Sep 17, 2024

関連URL

概要

  • FormControlにerrorMessagesを設定した場合、内部のinputを自動的にエラー表示に切り替えるロジックを修正します
  • これまではjsでコンポーネントを検索、型がsmarhtr-ui提供の入力要素と一致する場合、という判定だった
  • この方法では実際に利用されるとコンポーネントがラップされたりで型が違ってしまうことは多々有り、正しく紐づかないことが多かった
  • jsで対象要素内をquerySelectorで検索することでより確実に対象inputなどと結びつくようにしたい

変更内容

  • コンポーネントのerror propsでエラー表示切替をするのではなく、aria-invalid属性が設定されればエラー表示にするように修正

確認方法

  • diffを確認し、エラーの紐づけ方が変更されていることを確認する

@yagimushi yagimushi force-pushed the fix-form-control-error-attributes branch 2 times, most recently from df11112 to 7deeb8c Compare September 17, 2024 02:54
@yagimushi yagimushi force-pushed the fix-form-control-error-attributes branch from 7deeb8c to c402dbd Compare September 17, 2024 23:11
Copy link

pkg-pr-new bot commented Sep 18, 2024

Open in Stackblitz

pnpm add https://pkg.pr.new/kufu/smarthr-ui@4925

commit: a8eaf4c

@AtsushiM AtsushiM changed the title Fix form control error attributes feat: FormControlのエラー表示設定方法を変更し、より確実に設定されるように修正する Sep 18, 2024
@yagimushi yagimushi force-pushed the fix-form-control-error-attributes branch from ffbd058 to 5d5135c Compare September 19, 2024 05:39
@AtsushiM AtsushiM marked this pull request as ready for review September 19, 2024 05:47
@AtsushiM AtsushiM requested a review from a team as a code owner September 19, 2024 05:47
@AtsushiM AtsushiM requested review from Qs-F and masa0527 and removed request for a team September 19, 2024 05:47
@s-sasaki-0529
Copy link
Contributor

PR本文を埋めていただけるとレビューしやすくて助かりますー

@uknmr
Copy link
Collaborator

uknmr commented Sep 20, 2024

内容関係ないしまだ見てないんですが、レビュー前はガンガン rebase した方がレビューしやすいように思いました。

@AtsushiM
Copy link
Member Author

内容関係ないしまだ見てないんですが、レビュー前はガンガン rebase した方がレビューしやすいように思いました。

これが見たいならFiles Changed見れば良いのでrebaseする意味があまりないような?

Copy link
Contributor

@masa0527 masa0527 left a comment

Choose a reason for hiding this comment

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

問題ないと思います!
プロダクト側で発生していた紐づけがうまくいかなかった部分もこちらの修正で解消されてました 👍

@AtsushiM AtsushiM merged commit e16aa25 into master Sep 26, 2024
11 checks passed
@AtsushiM AtsushiM deleted the fix-form-control-error-attributes branch September 26, 2024 22:07
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.

4 participants