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

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

Closed
2 tasks done
ashishkrz opened this issue Jul 23, 2024 · 21 comments · Fixed by #17089 or #17118
Closed
2 tasks done

Comments

@ashishkrz
Copy link

Package

@carbon/react

Browser

Chrome

Package version

1.62.1

React version

17.0.2

Description

ComboBox has downshiftProps where we we used to utilize onStateChange function to make any change directly to Downshift Component by changing the state. Previously onStateChange used to receive Downshift component as 2nd argument, but now there is no way to receive the component, it is always undefined.

Reproduction/example

https://stackblitz.com/edit/github-v56j87?file=src%2FApp.jsx

Steps to reproduce

  1. Select one Item from ComboBox
  2. Now try to clear the selection using close icon, it is not getting cleared.
  3. Now if you try to open ComboBox and now try doing clear, it will get cleared.

So basically the issue is, when item is selected and ComboBox is closed (not expanded), then item not getting cleared when you click close icon.

Suggested Severity

Severity 2 = User cannot complete task, and/or no workaround within the user experience of a given component.

Application/PAL

Power Virtual Server

Code of Conduct

@guidari
Copy link
Contributor

guidari commented Jul 23, 2024

Just for context, in v1.57.0 that was working.

@ashishkrz
Copy link
Author

Just for context, in v1.57.0 that was working.

Checked just now, Yes it works on v1.57.0. But here also on 1st click it's not getting cleared sometimes. 2nd click onwards it works fine.

@ali-sadeghin
Copy link

I confirm too: it worked in 1.57.0 and totally broke in our UI from 1.58.0

@tay1orjones
Copy link
Member

I believe this is due to an incompatibility between useCombobox and <Downshift>. The <Downshift> signature is

function(changes: object, stateAndHelpers: object)

The useCombobox signature is

function(changes: object)

It sounds like this is a known/intentional incompatibility.

I believe the update to use useCombobox may have shipped in 1.58.0. I'm not sure if there is a workaround here. In the stackblitz you're accessing the input value, could you get the same via a ref? It's placed internally directly on the input

<input
disabled={disabled}
className={inputClasses}
type="text"
tabIndex={0}
aria-haspopup="listbox"
title={textInput?.current?.value}
{...getInputProps({
'aria-controls': isOpen ? undefined : menuProps.id,
placeholder,
ref: { ...mergeRefs(textInput, ref) },

@ashishkrz
Copy link
Author

@tay1orjones Could you please help on stackblitz, just by modifying things using ref like you suggested?

@clouria
Copy link

clouria commented Jul 29, 2024

I am also running into a similar issue to this. I am unable to use the onChange and stateReducer in the downshiftProps for <ComboBox /> which were working prior to v1.58.0 and seem to be related to the useCombobox change.

@ali-sadeghin
Copy link

I need the ComboBox to be created as open some times, I use:

  `const downshiftProps = { isOpen: this.state.open };`

This is what is broken for us.

@mateBe95
Copy link

bump up, its break main functionalities in our product which is broken. Please provide the fix @tay1orjones in the new version.

@tay1orjones tay1orjones self-assigned this Aug 1, 2024
@tay1orjones tay1orjones moved this from ⏱ Backlog to 🚦 In Review in Design System Aug 1, 2024
@github-project-automation github-project-automation bot moved this from 🚦 In Review to ✅ Done in Design System Aug 1, 2024
@ashishkrz
Copy link
Author

@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?

@ali-sadeghin
Copy link

@tay1orjones I tried 1.63.1 and unfortunately, it doesn't help. Still broken.

@tay1orjones tay1orjones reopened this Aug 5, 2024
@github-project-automation github-project-automation bot moved this from ✅ Done to ⏱ Backlog in Design System Aug 5, 2024
@tay1orjones tay1orjones moved this from ⏱ Backlog to 🏗 In Progress in Design System Aug 5, 2024
@tay1orjones
Copy link
Member

@ashishkrz @ali-sadeghin Thanks for validating this and letting us know. I see the fix only covers half the problem. Your downshiftProps now fully override any used within Combobox, but the other missing half (if I'm understanding correctly) is enabling you to modify downshift's internal state.

I think I've found a way to patch that gap. It may require some changes on your end, but they should be minor. I should have a PR up today and we can publish a patch release for it as soon as it's merged.

Sorry for the delay and I appreciate your patience here. How downshiftProps is used is opaque to us outside of reports like these. It's hard to know when or where we need to patch the downshift APIs as it shifts over time.

@github-project-automation github-project-automation bot moved this from 🏗 In Progress to ✅ Done in Design System Aug 9, 2024
@ali-sadeghin
Copy link

@tay1orjones is the fix planned to be built any time soon? We have a comming push to prod ...
Thanks!

@guidari
Copy link
Contributor

guidari commented Aug 9, 2024

Hey @ali-sadeghin
It was just release in the v1.63.2

@ali-sadeghin
Copy link

@tay1orjones @guidari Gave it a quick try and it looks just as good as before now 👍
Thanks a lot, will do more testing, but I think it's all good for us.

@ashishkrz
Copy link
Author

@tay1orjones @guidari I tried the version 1.63.2, it's still broken. Could you please check? Here is the stackblitz link for your ref which is using 1.63.2 but still it is not working.

@ashishkrz
Copy link
Author

@tay1orjones The behaviour is still similar, it's not working. Could you please help me on the same if I need to use it differently? @ali-sadeghin How it worked for you, I have created a stackblitz (https://stackblitz.com/edit/github-v56j87?file=src%2FApp.jsx) which is using latest version of carbon, could you please give it a try there and check once?

Our PowerVS team is constantly looking for the fix for the same, will be helpful if you update on same. Thanks!

@tay1orjones
Copy link
Member

tay1orjones commented Sep 18, 2024

@ashishkrz Sorry for the confusion, details were mentioned on the follow up PR:

If a consumer was using an action that is no longer available, such as setState, a functional equivalent can usually be found. This will require a code change in consuming projects.

- stateAndHelpers.setState({ inputValue: 'Item 1' });
+ downshiftActions.current.setInputValue('Item 1');

The PR also contained an updated version of your same example code you can reference.

@ashishkrz
Copy link
Author

@tay1orjones Selection works fine but onChange is not getting called after the item is selected from combobox. Could you please check once?

@ashishkrz
Copy link
Author

ashishkrz commented Dec 5, 2024

@tay1orjones I have updated the code on stackblitz https://stackblitz.com/edit/github-v56j87-igdhfb?file=src%2FApp.jsx for you reference to check it quickly.

The onChange is not getting called after a selection:

onChange={(e) => { console.info('HELLO', e); }}

@tay1orjones
Copy link
Member

This should be fixed in 1.72.0 - if you're still seeing a problem with this, please open a new issue. Thanks!

@ashishkrz
Copy link
Author

ashishkrz commented Dec 9, 2024

@tay1orjones Created issue for same #18212. Please take a look. Looking for quick resolution as customers are waiting for this fix. Thanks!

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