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: select option never use key in jsx declare usage #1889

Merged
merged 2 commits into from
Nov 1, 2023

Conversation

pointhalo
Copy link
Collaborator

@pointhalo pointhalo commented Nov 1, 2023

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Test Case
  • TypeScript definition update
  • Document improve
  • CI/CD improve
  • Branch sync
  • Other, please describe:

PR description

  1. Select 使用 JSX 传入 Option时,Option的 key 未被收集,从而导致在 renderOption 列表时,总是走到兜底逻辑中去,在少数场景下,会导致react 抛出列表项 key 重复 的 warning
    image

  2. 仅影响 JSX 传入 option 写法,若使用 props.optionList 传入后选项无该问题

  3. 需要区分 props.optionList 中带key,与 JSX 传入 option时带key两种情况。前者的key 在 onSelect、onChange回调入参里需要保留,后者不需要

Changelog

🇨🇳 Chinese

  • Fix: 修复 Select 使用 JSX 传入 Option时,Option传入的 key 未在渲染时生效的问题

🇺🇸 English

  • Fix: Fix the problem that when Select uses JSX to pass in Option, the key passed in Option does not take effect during rendering.

Checklist

  • Test or no need
  • Document or no need
  • Changelog or no need

Other

  • Skip Changelog

Additional information

Copy link

codesandbox-ci bot commented Nov 1, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 706c90f:

Sandbox Source
pr-story Configuration
Semi Design: Simple Story Configuration

<Select.Option label='3' value='2' key='abc'></Select.Option>
<Select.Option label='2' value='3' key='efg'></Select.Option>
<Select.Option label='4' value='5'></Select.Option>
<Select.Option label='5' value='4'></Select.Option>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

before:
image

after:
image

{ label: '1', value: '2', key: 'kkk' },
{ label: '2', value: '3', key: 'jjj' },
{ label: '3', value: '2' },
]}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

still same
image

@pointhalo
Copy link
Collaborator Author

image

key存进 state.optionList 中会导致 onSelect、onChange 等回调也带上 额外的 key。这里需要再处理一下。

Copy link

cypress bot commented Nov 1, 2023

Passing run #2006 ↗︎

0 226 10 0 Flakiness 0

Details:

Merge 706c90f into 9b1221c...
Project: semi-design Commit: 73cb299bcd ℹ️
Status: Passed Duration: 15:47 💡
Started: Nov 1, 2023 4:15 AM Ended: Nov 1, 2023 4:30 AM

Review all test suite changes for PR #1889 ↗︎

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9b1221c) 88.06% compared to head (73cb299) 88.03%.

❗ Current head 73cb299 differs from pull request most recent head 706c90f. Consider uploading reports for the commit 706c90f to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1889      +/-   ##
==========================================
- Coverage   88.06%   88.03%   -0.03%     
==========================================
  Files         436      436              
  Lines       25520    25522       +2     
  Branches     6440     6440              
==========================================
- Hits        22473    22469       -4     
- Misses       3047     3053       +6     
Files Coverage Δ
packages/semi-foundation/select/foundation.ts 81.74% <100.00%> (+0.02%) ⬆️
packages/semi-ui/select/index.tsx 84.32% <100.00%> (ø)
packages/semi-ui/select/utils.tsx 94.59% <100.00%> (+0.15%) ⬆️

... and 1 file with indirect coverage changes

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

@pointhalo pointhalo merged commit 6a1e34d into main Nov 1, 2023
7 checks passed
@pointhalo pointhalo deleted the fix-selectOptionKey branch November 1, 2023 05:58
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