Skip to content

fix: Ref was not being passed through for Picker#2122

Merged
mofojed merged 3 commits intodeephaven:mainfrom
mofojed:fix-picker-props
Jul 3, 2024
Merged

fix: Ref was not being passed through for Picker#2122
mofojed merged 3 commits intodeephaven:mainfrom
mofojed:fix-picker-props

Conversation

@mofojed
Copy link
Copy Markdown
Member

@mofojed mofojed commented Jul 3, 2024

  • Pass through ref to SpectrumPicker correctly
  • Added a useMultiRef hook for handling multiple refs being passed in

- Pass through ref to SpectrumPicker correctly
@mofojed mofojed requested a review from bmingles July 3, 2024 17:29
@mofojed mofojed self-assigned this Jul 3, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 3, 2024

Codecov Report

Attention: Patch coverage is 53.84615% with 6 lines in your changes missing coverage. Please review.

Project coverage is 46.59%. Comparing base (8fe9bad) to head (9ea8898).

Files Patch % Lines
packages/components/src/spectrum/picker/Picker.tsx 0.00% 4 Missing ⚠️
...ages/components/src/spectrum/comboBox/ComboBox.tsx 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2122   +/-   ##
=======================================
  Coverage   46.58%   46.59%           
=======================================
  Files         681      682    +1     
  Lines       38431    38441   +10     
  Branches     9762     9701   -61     
=======================================
+ Hits        17905    17912    +7     
+ Misses      20516    20477   -39     
- Partials       10       52   +42     
Flag Coverage Δ
unit 46.59% <53.84%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Comment thread packages/components/src/spectrum/picker/Picker.tsx Outdated
Copy link
Copy Markdown
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

Left a comment regarding the scroll ref

@mofojed mofojed requested a review from bmingles July 3, 2024 20:37
@mofojed
Copy link
Copy Markdown
Member Author

mofojed commented Jul 3, 2024

I tested this with DHE, looks to be working correctly.

Comment thread packages/components/src/spectrum/picker/useMultiRef.ts
Comment thread packages/components/src/spectrum/picker/useMultiRef.test.ts
bmingles
bmingles previously approved these changes Jul 3, 2024
Copy link
Copy Markdown
Contributor

@bmingles bmingles 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. Left question about additional test case but I wouldn't hold up PR for it.

Copy link
Copy Markdown
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

LGTM

@mofojed mofojed enabled auto-merge (squash) July 3, 2024 22:26
@mofojed mofojed merged commit a11e2ce into deephaven:main Jul 3, 2024
@mofojed mofojed deleted the fix-picker-props branch July 3, 2024 22:34
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants