Skip to content

fix(dropdown): spread dropdownProps last #17089

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

Merged

Conversation

tay1orjones
Copy link
Member

@tay1orjones tay1orjones commented Aug 1, 2024

Closes #17027

Changelog

Changed

  • Update downshift-based components to spread ...dropdownProps last so that consumers can fully configure downshift's behavior
  • Update docs and prop/types comments to more clearly state this and the associated pitfalls

Testing / Reviewing

  • Review downshift-based components in storybook, they should be unchanged

Copy link

netlify bot commented Aug 1, 2024

Deploy Preview for v11-carbon-react ready!

Name Link
🔨 Latest commit 6405aea
🔍 Latest deploy log https://app.netlify.com/sites/v11-carbon-react/deploys/66abe8910562bf0008f79286
😎 Deploy Preview https://deploy-preview-17089--v11-carbon-react.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Aug 1, 2024

Deploy Preview for carbon-elements ready!

Name Link
🔨 Latest commit 6405aea
🔍 Latest deploy log https://app.netlify.com/sites/carbon-elements/deploys/66abe891d8360d0008b04e7f
😎 Deploy Preview https://deploy-preview-17089--carbon-elements.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@emyarod emyarod left a comment

Choose a reason for hiding this comment

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

looks good to me!

@guidari guidari added this pull request to the merge queue Aug 1, 2024
@tay1orjones tay1orjones removed this pull request from the merge queue due to a manual request Aug 1, 2024
@tay1orjones tay1orjones merged commit 0ddee05 into carbon-design-system:main Aug 1, 2024
22 checks passed
tay1orjones added a commit to tay1orjones/carbon that referenced this pull request Aug 1, 2024
@ashishkrz
Copy link

[FIX NOT WORKING] @tay1orjones Just to confirm what is changed now? I see that the position of spreaded downshiftProps got changed but that will not fix this issue, I believe. Still the 2nd argument to onStateChange function remains undefined.

Could you please help over stackblitz https://stackblitz.com/edit/github-v56j87?file=src%2FApp.jsx making it work with the fix done?

@mateBe95
Copy link

mateBe95 commented Aug 2, 2024

Yes fix seems to be not working. Same as @ashishkrz +1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: ComboBox downshiftProps missing reference to Downshift component where state change can be done
5 participants