-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat(chartGraph): convert graph data, add error/loading states #40
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made a commit with the internal review changes. Give it once over. We probably need to
- look at moving the x and y axis defaults into the helpers due to needing additional logic in the event a valid array result set is returned with zero results...
- also we may need to look at filling out a full 30 days in the event a partial array result set is returned.. but we can actually test that against the API through the proxy
@@ -246,125 +246,125 @@ exports[`RhelGraphCard Component should render a non-connected component: non-co | |||
exports[`RhelGraphCard Component should render multiple states: error shows zeroed bar values 1`] = ` | |||
Object { | |||
"chartBarData": Array [ | |||
Object { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'm good after this bit is updated... i believe it should start w/ Jun 1 if Jun 1 UTC is passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ugh missed that...
edit:
it's part of the conversion that's happening as the default props get replaced. sooooo it looks like that needs to be offset inside the helpers if we want to continue passing a date
type... otherwise we'll end up being inconsistent and sometimes passing an instance of moment
or in the case of what needs to happen for the component an instance of date
... I'll refactor the helpers so the moment aspect is contained and the component is merely a passthrough
edit:
so technically the output of this is accurate, that's what is produced based off of a new Date('2019-06-01T00:00:00Z')
, and when run in our environments a local output ... I was silly, and missed a utc
conversion in the graphHelpers, hence the offset was being included
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahh... yes, you are correct:
new Date('2019-06-01T00:00:00Z')
Fri May 31 2019 20:00:00 GMT-0400 (Eastern Daylight Time)
Ok, yes, if sourced from the server....shouldn't matter. I am guessing that Jest uses the system UTC time (so EST in my case), but not positive.
Maybe some way to mock this better in the future? FWIW, it's not obvious. Could also just set to something same day in UTC /U.S. time zones for being more obvious...
new Date('2019-06-01T12:00:00Z')
Sat Jun 01 2019 08:00:00 GMT-0400 (Eastern Daylight Time)
8514db0
to
ba10156
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests are passing, UTC implementation appears to be behaving on date render. I'm good if you're good.
Double check your commit message when you're ready, squash em, and make sure to update your commit subject with the issues/18
or the PR number, or both. Looks like when the squash was disabled that was nixed too, at least on my last commit...
feat(chartGraph): convert graph data, add error/loading states (#40)
…tInsights#40) * parse the Redux date and sockets inputs and generate chart data and tooltips * add error state and zeroed array * loading skeletor * graphHelpers, rhelGraphCard replace translate refs * rhelGraphCard, consolidate logic * update integration snapshots * graphHelpers, minor UTC correction, snapshot corrections * dateHelpers, initial date defaults, prep for additional date functions * helpers, index layout * rhelGraphCard, replace default startDate, endDate props with helper
Codecov Report
@@ Coverage Diff @@
## master #40 +/- ##
==========================================
+ Coverage 84.55% 85.81% +1.26%
==========================================
Files 22 23 +1
Lines 246 275 +29
Branches 53 59 +6
==========================================
+ Hits 208 236 +28
- Misses 30 31 +1
Partials 8 8
Continue to review full report at Codecov.
|
What's included
tooltips
Outstanding question - should a max-y value be set for non error state?
How to test
Example
Loading desktop:
Loading mobile:
Loading complete:
Error state (zeroed array), max-y set to 100, toast notification to come in future PR... :
Updates issue/story
Closes #18