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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 3 commits into from
Jul 3, 2024
Merged

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

merged 3 commits into from
Jul 3, 2024

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Jul 3, 2024 •
edited
Loading

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

@mofojed
fix: Ref was not being passed through for Picker
718b1f7 
- 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 Codecov
Copy link

codecov bot commented Jul 3, 2024 •
edited
Loading

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.

bmingles
bmingles requested changes Jul 3, 2024
View reviewed changes
Copy link
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
Member Author

mofojed commented Jul 3, 2024

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

bmingles
bmingles previously approved these changes Jul 3, 2024
View reviewed changes
Copy link
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.

@mofojed mofojed requested a review from bmingles July 3, 2024 22:22
bmingles
bmingles approved these changes Jul 3, 2024
View reviewed changes
Copy link
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
11 checks passed
@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.
Reviewers

@bmingles bmingles bmingles approved these changes

Assignees

@mofojed mofojed

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mofojed @bmingles