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

Correct forwarded refs #389

Merged
merged 7 commits into from
Oct 30, 2023
Merged

Correct forwarded refs #389

merged 7 commits into from
Oct 30, 2023

Conversation

martimalek
Copy link
Contributor

@martimalek martimalek commented Oct 6, 2023

What:

Make refs point to the most obvious element in Input and Password components and correct refs usages in our components. Apart from that I've also removed the forwardRef from our internal Datepicker component since it wasn't being used (this is not a component exposed by Wave, it's used internally by DatepickerSingleInput and DatepickerRangeInput)


Why:

To correct our usages of refs so that it follows our agreement.


How:

  • Correct ref type in Input, Password and InnerInput to HTMLInputElement
  • Correct the ref in InnerInput so it's passed to the input instead of to the wrapper
  • Adapt DatepickerRangeInput to use the new Input ref
  • Adapt PhoneInput to use the new Input ref
  • Remove forwardRef from Datepicker


Other:

After this PR we are only forwarding refs in 3 places in the codebase:

  • Input and Password components (external components that our users can use)
  • InnerInput component (internal component that we don't expose, needed for Input and Password)

Since the ref in the Datepicker (internal) was not being used in either of it's parents (the actual Datepicker and DatepickerRangeInput components that we expose) there was no way for a project to depend on this ref, so I've decided to remove it. Even if in the future we decide to have a ref/refs in Datepicker it will point to a different element, since the one removed did not follow our agreement on refs.

Checklist:

  • Check ref in internal Datepicker
  • Ready to be merged

Closes #169

@martimalek martimalek self-assigned this Oct 6, 2023
@martimalek martimalek marked this pull request as ready for review October 25, 2023 16:28
@martimalek martimalek merged commit 5b95032 into next Oct 30, 2023
10 checks passed
@martimalek martimalek deleted the correct-forwarded-refs branch October 30, 2023 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants