search

deephaven / web-client-ui

Deephaven Web Client UI
Apache License 2.0
29 stars 31 forks source link

feat: checkbox_group re-export #2212

Closed AkshatJawne closed 1 month ago

AkshatJawne commented 2 months ago

Resolves #2211

Needed for changes in plugins PR: https://github.com/deephaven/deephaven-plugins/pull/813

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 22.22222% with 7 lines in your changes missing coverage. Please review.

Project coverage is 46.64%. Comparing base (ec9b41e) to head (983f35e). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/components/src/spectrum/CheckboxGroup.tsx 0.00% 7 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #2212 +/- ## ========================================== + Coverage 46.61% 46.64% +0.03% ========================================== Files 692 694 +2 Lines 38508 38517 +9 Branches 9826 9804 -22 ========================================== + Hits 17949 17965 +16 + Misses 20506 20499 -7 Partials 53 53 ``` | [Flag](https://app.codecov.io/gh/deephaven/web-client-ui/pull/2212/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deephaven) | Coverage Δ | | |---|---|---| | [unit](https://app.codecov.io/gh/deephaven/web-client-ui/pull/2212/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deephaven) | `46.64% <22.22%> (+0.03%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=deephaven#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AkshatJawne commented 2 months ago

Cannot verify in styleguide as labels are currently blank due to latest core change @bmingles , will update and add examples as soon as that is fixed

bmingles commented 2 months ago

Cannot verify in styleguide as labels are currently blank due to latest core change @bmingles , will update and add examples as soon as that is fixed

@AkshatJawne I don't think those are related. The labels that are missing are in the ListView right?

AkshatJawne commented 2 months ago

Ironed out comments, not sure why the String(child) is resulting in booleans resulting in null showing up as the text, @bmingles any ideas?

AkshatJawne commented 2 months ago

Should be good now, I still had to clone element, or else when we passed in Checkbox children, then the value would not be assigned properly, and thus selecting one would select all of them. @bmingles

bmingles commented 2 months ago

I think the unit test failures are at least in part to needing a rebase / merge on latest main.

bmingles commented 2 months ago

@AkshatJawne Code looks good. I think e2e tests may need to be updated.

AkshatJawne commented 1 month ago

Attempting to update snapshots right now, running into some difficulty, will update them soon hopefully.