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: Loaderの表示遅延処理を削除する #5003

Merged
merged 1 commit into from
Oct 15, 2024

Conversation

AtsushiM
Copy link
Member

@AtsushiM AtsushiM commented Oct 11, 2024

関連URL

概要

  • Loaderの表示遅延処理を削除します

Why?

  • 現在のロジックは 表示するタイミングを0.2secずらす ことしかできていない
  • これにより、0.3sec ~ 0.4sec のロード時間がかかった場合、逆に本来はチラつかなかったローディング表示がちらついてしまう
  • 上記のような状態だったため ちらつきが増加しているのか、減少しているのか がわからないコードになってしまっていた
  • そもそもローディング時間はユーザー環境で可変してしまうため ローディング時間がかからない箇所だからLoaderは使わないでおこう という判断は開発者視点でみた場合、行うことはできない
  • 上記からコード的な意味が薄く、混乱を招きかねないため削除する事になりました
  • もし優位にチラつきが増えた場合、別途対応を検討します
    • 現在有力なのは 0.2secほどローディングを強制的に表示し続ける方法になりそうです

変更内容

確認方法

@yagimushi yagimushi force-pushed the remove-loader-defer-display branch from eb687e0 to 46f044f Compare October 11, 2024 08:10
Copy link

pkg-pr-new bot commented Oct 11, 2024

Open in Stackblitz

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

commit: 46f044f

@AtsushiM AtsushiM marked this pull request as ready for review October 14, 2024 22:10
@AtsushiM AtsushiM requested a review from a team as a code owner October 14, 2024 22:10
@AtsushiM AtsushiM requested review from Qs-F and masa0527 and removed request for a team October 14, 2024 22:10
Copy link
Collaborator

@uknmr uknmr left a comment

Choose a reason for hiding this comment

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

File changed は LGTM!
意思決定に関わった人しか理解できないので、消すに至った経緯などを PR 本文に書いておいて欲しいです!

@uknmr uknmr enabled auto-merge (squash) October 15, 2024 03:40
@uknmr uknmr merged commit 8025cc3 into master Oct 15, 2024
15 checks passed
@uknmr uknmr deleted the remove-loader-defer-display branch October 15, 2024 03:40
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