concord-consortium / codap

CODAP (Common Online Data Analysis Platform)
MIT License
94 stars 38 forks source link

Basic date-time axis #1398

Closed bfinzer closed 3 weeks ago

bfinzer commented 4 weeks ago

[#181971361] Feature: Date attributes can be plotted on a date-time axis

This is a "first pass" implementation of a date-time axis. Missing features and bugs should be logged in separate PT stories unless they break non-date-time axes or otherwise disrupt normal V3 behavior.

codecov[bot] commented 4 weeks ago

Codecov Report

Attention: Patch coverage is 43.16310% with 345 lines in your changes missing coverage. Please review.

Project coverage is 71.47%. Comparing base (7372552) to head (07e811e). Report is 1 commits behind head on main.

Files Patch % Lines
.../components/axis/helper-models/date-axis-helper.ts 0.98% 303 Missing :warning:
...3/src/components/graph/models/graph-model-utils.ts 26.66% 11 Missing :warning:
v3/src/components/axis/hooks/use-axis.ts 83.33% 8 Missing :warning:
v3/src/components/graph/components/scatterdots.tsx 0.00% 4 Missing :warning:
v3/src/utilities/date-utils.ts 33.33% 4 Missing :warning:
v3/src/components/axis/hooks/use-sub-axis.ts 88.00% 3 Missing :warning:
v3/src/components/graph/utilities/graph-utils.ts 25.00% 3 Missing :warning:
v3/src/components/axis/axis-utils.ts 33.33% 2 Missing :warning:
...omponents/data-display/data-display-value-utils.ts 71.42% 2 Missing :warning:
...nts/movable-value/movable-value-adornment-model.ts 50.00% 2 Missing :warning:
... and 2 more

:exclamation: There is a different number of reports uploaded between BASE (7372552) and HEAD (07e811e). Click for more details.

HEAD has 1 upload less than BASE | Flag | BASE (7372552) | HEAD (07e811e) | |------|------|------| |jest|1|0|
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1398 +/- ## =========================================== - Coverage 85.89% 71.47% -14.42% =========================================== Files 533 532 -1 Lines 26094 23162 -2932 Branches 6684 6953 +269 =========================================== - Hits 22413 16555 -5858 - Misses 3526 6422 +2896 - Partials 155 185 +30 ``` | [Flag](https://app.codecov.io/gh/concord-consortium/codap/pull/1398/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | Coverage Δ | | |---|---|---| | [cypress](https://app.codecov.io/gh/concord-consortium/codap/pull/1398/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `71.47% <43.16%> (-0.97%)` | :arrow_down: | | [jest](https://app.codecov.io/gh/concord-consortium/codap/pull/1398/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=concord-consortium) | `?` | | 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=concord-consortium#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.

cypress[bot] commented 4 weeks ago



Test summary

204 0 29 0Flakiness 0


Run details

Project codap-v3
Status Passed
Commit 50fdc6d2f6
Started Aug 16, 2024 7:25 PM
Ended Aug 16, 2024 7:33 PM
Duration 08:25 💡
OS Linux Ubuntu -
Browser Chrome 127

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

kswenson commented 3 weeks ago

The other thing I forgot to mention in my review, is that we're going to want some tests. If we don't want to hold up this PR, then we should plan to come back to them. There are a number of utility functions that should be straightforward to test with jest tests and then we should have a cypress test that brings up a date axis.

kswenson commented 3 weeks ago

@bfinzer I see that there's a failing jest test which should be addressed before merging as well.