Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

[WIP]feat(*): developer focused demo using StoryBook - I315 #335

Closed

Conversation

elit-altum
Copy link
Contributor

@elit-altum elit-altum commented Mar 10, 2020

Signed-off-by: elit-altum manan.sharma311@gmail.com

Issue #315

Changes

  • Currently added @storybook/react as a dev dependency and script to start storybook.
  • This is currently for staging purpose

Related Issues

Signed-off-by: elit-altum <manan.sharma311@gmail.com>
@elit-altum
Copy link
Contributor Author

@sanyamdogra I think we can start from here.
Please go through https://stackoverflow.com/questions/41588025/git-github-push-to-someone-elses-fork-of-my-repo. I think this is a good way to collaborate on a single PR. What do you say?

@elit-altum
Copy link
Contributor Author

@irmerk needed your thoughts on some initial settings

  1. Where should we write and store the stories, as in what directory should we use? Storybook recommends co-locating in the src folder itself.
  2. Is the script to run storybook okay? I used the recommended npm run storybook

@sanyamdogra
Copy link
Contributor

@sanyamdogra I think we can start from here.
Please go through https://stackoverflow.com/questions/41588025/git-github-push-to-someone-elses-fork-of-my-repo. I think this is a good way to collaborate on a single PR. What do you say?

Well I don't think I'll be comfortable on working on a same PR. It'll lead to discrepancies in future. This might make sense have made sense if we use the same fork.

@sanyamdogra
Copy link
Contributor

@irmerk needed your thoughts on some initial settings

  1. Where should we write and store the stories, as in what directory should we use? Storybook recommends co-locating in the src folder itself.
  2. Is the script to run storybook okay? I used the recommended npm run storybook
  1. We will include a . stories.js for every component present in the project.
  2. Yes npm run storybook looks good to me.

@elit-altum
Copy link
Contributor Author

@sanyamdogra I think we can start from here.
Please go through https://stackoverflow.com/questions/41588025/git-github-push-to-someone-elses-fork-of-my-repo. I think this is a good way to collaborate on a single PR. What do you say?

Well I don't think I'll be comfortable on working on a same PR. It'll lead to discrepancies in future. This might make sense have made sense if we use the same fork.

Unsure of what you asked for. Are you referring to collaborator access to a fork? Yes I will have to provide you that on my fork.

@jolanglinais
Copy link
Member

I think working off a fork and building a draft PR from that fork would be best.

Script seems fine. And I would presume to follow what they say in their docs, so colocating sounds fine.

@sanyamdogra
Copy link
Contributor

sanyamdogra commented Mar 11, 2020

@sanyamdogra I think we can start from here.
Please go through https://stackoverflow.com/questions/41588025/git-github-push-to-someone-elses-fork-of-my-repo. I think this is a good way to collaborate on a single PR. What do you say?

Well I don't think I'll be comfortable on working on a same PR. It'll lead to discrepancies in future. This might make sense have made sense if we use the same fork.

Unsure of what you asked for. Are you referring to collaborator access to a fork? Yes I will have to provide you that on my fork.

Alright then. Provide me access to your fork. I hope it doesn't affect mine. Let us also discuss about a roadmap but on this issue #315.

jeromesimeon and others added 10 commits March 12, 2020 01:39
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: irmerk <jolenelanglinais@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
@elit-altum
Copy link
Contributor Author

@sanyamdogra fixed the failing builds. Separated other's commits for easy squashing at the end.

Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
@sanyamdogra
Copy link
Contributor

Hi @elit-altum, can you please fix the build? After that, I guess initial setup of the Storybook is done. We can get it reviewed by the team. I have added dummy stories that can be tackled one at a time.

Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: Sanyam Dogra <sdlord07@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
Signed-off-by: elit-altum <manan.sharma311@gmail.com>
@sanyamdogra
Copy link
Contributor

@elit-altum I guess you'll have to change the directed PR to development rather than master.

@elit-altum elit-altum changed the base branch from master to development March 17, 2020 13:08
@jolanglinais
Copy link
Member

Screenshots:


Screen Shot 2020-03-17 at 1 24 39 PM


Screen Shot 2020-03-17 at 1 24 57 PM

Copy link
Member

@DianaLease DianaLease left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good starting point! What are the next steps? With this in place, people will be able to go ahead and update the individual .stories files and add new .stories files for components that don't get have one?

@sanyamdogra
Copy link
Contributor

This looks like a good starting point! What are the next steps? With this in place, people will be able to go ahead and update the individual .stories files and add new .stories files for components that don't get have one?

Yes, we'll be able to write stories for every component that is used in cicero-ui.They can be checked in isolation and even tested. We'll still have to write stories for these components but that can be covered in individual PRs.

@DianaLease
Copy link
Member

This looks like a good starting point! What are the next steps? With this in place, people will be able to go ahead and update the individual .stories files and add new .stories files for components that don't get have one?

Yes, we'll be able to write stories for every component that is used in cicero-ui.They can be checked in isolation and even tested. We'll still have to write stories for these components but that can be covered in individual PRs.

Sounds great to me!
It looks like some of your commits are missing the DCO signoff. You'll have to update your PR so that there no no commits without the signoff.

@sanyamdogra
Copy link
Contributor

This looks like a good starting point! What are the next steps? With this in place, people will be able to go ahead and update the individual .stories files and add new .stories files for components that don't get have one?

Yes, we'll be able to write stories for every component that is used in cicero-ui.They can be checked in isolation and even tested. We'll still have to write stories for these components but that can be covered in individual PRs.

Sounds great to me!
It looks like some of your commits are missing the DCO signoff. You'll have to update your PR so that there no no commits without the signoff.

Me and @elit-altum tried hard to fix the build. The last commit has the DCO, somehow the travis is not building.

@sanyamdogra
Copy link
Contributor

I don't know how it messed up the package-lock.json. Me and @elit-altum have tried to fix this but it's all scrambled. Closing this PR and making a new one.

@jolanglinais
Copy link
Member

Closing for #353, @elit-altum make sure that PR isn't missing anything from this one.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants