Demo info not rendering #234 #238
Conversation
stories/dem-info.stories.jsx
Outdated
| @@ -8,7 +8,7 @@ export default { | |||
|
|
|||
| export const PrimaryAllFieldsIncluded = { | |||
| args: { | |||
| dob: '1990-10-20T00:00:00.000Z', | |||
| dob: '1990-10-20', | |||
There was a problem hiding this comment.
The test data is correct, it's an ISO format date string. If necessary please fix the code to work with the data in the story.
There was a problem hiding this comment.
Something is weird in this PR. There are lines missing, but they aren't showing up in the diff from master. Also the export it missing, but not showing up in the diff.
Can you 'git fetch' and 'git merge origin/master' and resolve the conflicts, and make sure it looks right.
Did you at any time do a rebase? If so, let me know and lets figure out what to do.
app/components/dem-info.jsx
Outdated
| @@ -5,10 +5,12 @@ import insertSheet from 'react-jss' | |||
|
|
|||
| function DemInfo(props) { | |||
| const { state, dob, party, classes, className, ...otherProps } = props | |||
There was a problem hiding this comment.
classes is not a prop, it is create by:
const classes = useStylesFromThemeFunction()
|
Hello David, |
There was a problem hiding this comment.
The merge and new push fixed the problems I was seeing, I don't understand why it happened though.
I see that the change to using new Date(...).toString() didn't really fix the behavior of storybook.
I even copied an iso date string into the party field and got the same soft of behavior - it doesn't show the text anymore, it just says edit. But you can put in new stuff and it works.
Since the change didn't really effect anything, and we go back to just using the strings. as before. I worry that future me, or some new looking at this will think that something has to be done to the data coming out of the db.
|
Hello David, If I use dob: new Date('1990-10-20T00:00:00.000Z') to replace dob: new Date('1990-10-20T00:00:00.000Z').toISOString(), the behavior in Storybook will be fixed (see Figure 1). Could we proceed with this approach? Also, there are a couple of issues in the demo-info code in the master branch. The line const { state, dob, party, classes, className, ...otherProps } = props; incorrectly creates the class variable.
|
There was a problem hiding this comment.
for dem-info.stories.jsx what I'm asking for is essentially to revert the file back to what it was originally. Where dob is assigned a string. '1990-10-20T00:00:00.000Z', I wasn't asking to take of the .toString() I was asking to get ride for the whole new Date( part. The story needs to emulate what the parent component is going to pass to the component in the integrated application.
the comment about classes in props in master was part of the weird behavior. It's not like that in origin/master. Perhaps some changes got unintentionally committed to the local copy of master on your machine. But the PR online looks fine at this poiint: https://github.com/EnCiv/civil-pursuit/pull/238/files
That this case http://localhost:6006/?path=/story/dem-info--party-only-empty-field returns null is correct. There is nothing to render.
|
Hello David, Thank you for the clarification. I'm a bit unclear about the next steps. Should I make further changes to the PR, or just proceed with https://github.com/EnCiv/civil-pursuit/pull/238/files? Thanks! |
|
That is the PR. If you push changes to your branch they will show up there. See #238. |
There was a problem hiding this comment.
Please look at the PR, https://github.com/EnCiv/civil-pursuit/pull/238/files - there are no real changes any more.
I was trying to get you to revert the changes to the story file, but not to the component.
#234