fix(dropdown): broken forwardRef binding

[ConradSchmidt]

Closes #7269

Put ref prop at the end, so that it is not overwritten by other props.

Changelog

New

Changed

• fixed broken forwardRef for dropdown and multiselect

Removed

Testing / Reviewing

1. Add ref to dropdown/multiselect
   e.g. in the story:

  const ref = createRef();
  useEffect(() => {
    console.log(ref);
  }, [ref]);
...
<MultiSelect ref={ref} ...

2. Open console and wait for {current: null}
3. Apply purposed fix
4. Open console and wait for current: button#downshift-5-toggle-button.bx--list-box__field}

[netlify]

Deploy preview for carbon-elements ready!

Built with commit f9099a1

https://deploy-preview-7270--carbon-elements.netlify.app

[ConradSchmidt]

Is there a reason why the ref is bound to the button? I guess the dropdown (container) itself would be a better candidate.

[netlify]

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit f9099a1

https://deploy-preview-7270--carbon-components-react.netlify.app

[joshblack]

For the ref placement itself it is likely on the button to help with focus management (e.g. a consumer can programmatically focus the button).

Since the getButtonProps helper adds in a ref from downshift, will we need to merge the two refs potentially? 👀

[ConradSchmidt]

How should this approach this? Options I see:

1. leave it like this (I didn't experienced different behaviour)
2. use the wrapper container (different element) as ref
3. try to merge refs (no clue how :D)
4. ?

[joshblack]

Great question @ConradSchmidt! I think you can use the mergeRefs helper that we have: https://github.com/carbon-design-system/carbon/blob/f1c92af3f40585631d8450a4aa381d121dc2c797/packages/react/src/tools/mergeRefs.js

I was trying to dig into that getToggleButtonProps method real quick over on downshift but I couldn't find where it was setting ref. Would you happen to have a link handy? 👀

[ConradSchmidt]

@joshblack like this: https://github.com/carbon-design-system/carbon/pull/7270/files#diff-9712440dfce10e10be2619b0921a1ba53e775f5d864aecad652354f4e4d198b1R183?
I can not really verify if this actually works, because even when I overwrite the downshift ref it is working fine. The merging works for the passed in ref, which is a good sign.

[ConradSchmidt]

I guess I stop the package installation here, maybe @joshblack can take care of this

[joshblack]

Hey @ConradSchmidt! So sorry about the delay, I pushed up some changes that I think reflect what you were going for, let me know if that works!

[andreancardona]

I still see a failing CI :(

[joshblack]

@andreancardona should be updated now!

[yuecchen]

which version this fix will be go into?

or is there any workaround we can do it overcome this issue?

[andreancardona]

looks good!