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

chore: eslintのignore commentを削除 or 不要なように調整する #4986

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

AtsushiM
Copy link
Member

@AtsushiM AtsushiM commented Oct 8, 2024

関連URL

概要

  • eslintのignore commentが開発時にノイズになるため、必要最小限になるよう、調整
  • その際、既に不要になっていた箇所もあったため削除なども実施

変更内容

確認方法

  • 差分をみてロジックに影響がないことを確認する

Copy link

pkg-pr-new bot commented Oct 8, 2024

Open in Stackblitz

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

commit: 8d943e3

@AtsushiM AtsushiM changed the title Fix eslint chore: eslintのignore commentを削除 or 不要なように調整する Oct 8, 2024
Comment on lines +48 to +51

if (onFormatValue) {
onFormatValue(formatted)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

今回の修正とは本質的に関係がないのですが、何故かコメント削除するとエラーが発生するようになったため、厳密な書き方に修正しています

Copy link
Contributor

Choose a reason for hiding this comment

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

この件ですねー。実害はないので良さそう。
https://kufuinc.slack.com/archives/CGC58MW01/p1728349717774749

@@ -145,18 +145,12 @@ export const Demo: StoryFn = () => {
)}
<StyledWrapper>
<Stack>
{/* eslint-disable-next-line jsx-a11y/label-has-associated-control */}
<label>
Copy link
Member Author

Choose a reason for hiding this comment

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

CheckBoxは内部でlabelを持っており、label in label になっていたため削除しています。
マークアップ構造が変わっていますが、storybookのため、ユーザー影響はありません

@AtsushiM AtsushiM marked this pull request as ready for review October 8, 2024 05:27
@AtsushiM AtsushiM requested a review from a team as a code owner October 8, 2024 05:27
@AtsushiM AtsushiM requested review from oti and yt-ymmt and removed request for a team October 8, 2024 05:27
@@ -173,5 +172,7 @@ export const Input = forwardRef<HTMLInputElement, Props & ElementProps>(

const disableWheel = (e: WheelEvent) => {
// wheel イベントに preventDefault はないため
e.target && (e.target as HTMLInputElement).blur()
if (e.target) {
;(e.target as HTMLInputElement).blur()
Copy link
Contributor

Choose a reason for hiding this comment

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

[nits]
このセミコロンは流石に無くても大丈夫そう

Copy link
Member Author

Choose a reason for hiding this comment

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

なんかformatで自動追加されたんですよね〜
まぁこのまま行きますね

Copy link
Contributor

@s-sasaki-0529 s-sasaki-0529 left a comment

Choose a reason for hiding this comment

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

👍️ ESLint 周りの対応以外も、コンポーネントの挙動に影響を与えるものではないことを確認しましたー。

@AtsushiM AtsushiM merged commit 77c3a67 into master Oct 8, 2024
29 of 30 checks passed
@AtsushiM AtsushiM deleted the fix-eslint branch October 8, 2024 06:17
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