Skip to content

fix: guard onClick invocation when Steps has no onChange - #400

Merged
yoyo837 merged 1 commit into
react-component:masterfrom
feritcemkahraman:fix/step-onclick-fallback
Oct 22, 2026
Merged

yoyo837 merged 1 commit into
react-component:masterfrom
feritcemkahraman:fix/step-onclick-fallback

Conversation

@feritcemkahraman

@feritcemkahraman feritcemkahraman commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

When is rendered without an onChange prop, props.onClick passed to is undefined (onClick={onChange && onStepClick}).

In Step.tsx, clickable is computed as: !!(onClick || onItemClick) && !disabled;

If an item defines an item-level onClick (onItemClick), clickable evaluates to true. However, accessibilityProps.onClick unconditionally invoked onClick(index), resulting in a TypeError: onClick is not a function runtime crash when Steps does not supply an onChange callback.

Fix

  • Mark onClick?: (index: number) => void as optional in StepProps.
  • Use optional chaining onClick?.(index) before invoking.
  • Add unit test in tests/index.test.tsx.

Validation

  • pnpm test (All 25 unit tests and 13 snapshots passing)
  • pnpm tsc (Clean tsc --noEmit)
  • pnpm lint (0 errors)
  • pnpm compile (Clean father build ESM/CJS)

Summary by CodeRabbit

  • Bug 修复
    • 未提供步骤变更回调时,点击步骤仍可正常触发该步骤自身的点击回调。
  • 测试
    • 增加了未提供步骤变更回调时的点击行为覆盖,并更新了禁用项的回调断言。

@vercel

vercel Bot commented Oct 22, 2026

Copy link
Copy Markdown

@feritcemkahraman is attempting to deploy a commit to the afc163's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: a4faea90-600d-44f7-b98e-942a940d80d4
📥 Commits

Reviewing files that changed from the base of the PR and between 400f7cf and ac6aedb.

📒 Files selected for processing: 2
  • src/Step.tsx
  • tests/index.test.tsx

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

步骤的顶层 onClick 属性现在是可选的。组件在调用该回调前检查其是否存在。测试覆盖未提供 Steps.onChange 时步骤自身回调的调用。

Changes

步骤点击回调

Layer / File(s) Summary
可选回调与回归验证
src/Step.tsx, tests/index.test.tsx
StepProps.onClick 改为可选,调用时使用可选调用。新增测试验证未提供 Steps.onChange 时步骤自身的 onClick 会被调用。禁用步骤测试改用 not.toHaveBeenCalled(),仍验证 onChange 未被调用。

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix

Suggested reviewers: qdyanbing

Merge Risk: ⚪ Minimal · up to ac6ae

The change guards the absent callback while preserving step-level click behavior, and the supplied regression test covers that case. No actionable merge-blocking risk is evident.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed 标题准确概括了主要变更:当 Steps 没有 onChange 时,防止调用未定义的 onClick。
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

小兔轻点步骤旁,
回调可选不慌张。
没有总回调也无妨,
单项点击有回响。
测试守在春风旁。

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (400f7cf) to head (ac6aedb).

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #400   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines           97        97           
  Branches        36        37    +1     
=========================================
  Hits            97        97           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yoyo837
yoyo837 merged commit d1060e2 into react-component:master Oct 22, 2026
9 of 10 checks passed
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