Skip to content
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

Storybook components #743

Merged
merged 10 commits into from
Nov 17, 2019
Merged

Storybook components #743

merged 10 commits into from
Nov 17, 2019

Conversation

caiokf
Copy link
Contributor

@caiokf caiokf commented Nov 4, 2019

Adding stories for other components on Storybook. Covered in this PR:

  • Text
  • Heading
  • Icon
  • EventCard

@caiokf caiokf self-assigned this Nov 4, 2019
@cypress
Copy link

cypress bot commented Nov 6, 2019



Test summary

30 0 10 0


Run details

Project onearmy-community-platform
Status Passed
Commit 6c15879
Started Nov 17, 2019 3:26 PM
Ended Nov 17, 2019 3:35 PM
Duration 08:45 💡
OS Linux Ubuntu Linux - 14.04
Browser Electron 73

View run in Cypress Dashboard ➡️


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 Dashboard

@@ -36,6 +36,13 @@ export const uppercase = props =>
}
: null

export const capitalize = props =>
Copy link
Contributor

@drydenwilliams drydenwilliams Nov 7, 2019

Choose a reason for hiding this comment

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

🤔 Not your work but this structure of adding styles seems a bit messy and not super easy to read to me.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes some components needs refactoring, especially Text I've addressed the issue here #727

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@drydenwilliams
Check this commit and see if this looks a bit better (would look nicer without lint breaking lines, but maybe still ok)

@caiokf caiokf force-pushed the storybook-components branch from f107206 to 8623ee0 Compare November 14, 2019 13:44
@caiokf caiokf changed the title WIP: Storybook components Storybook components Nov 14, 2019
@caiokf caiokf requested a review from BenGamma November 14, 2019 15:33
@caiokf
Copy link
Contributor Author

caiokf commented Nov 14, 2019

@BenGamma Please take a look, I managed to include a few more components, although not a comprehensive list yet. There are a few commits that touch some of the component's code, so please check those too.

Copy link
Contributor

@BenGamma BenGamma left a comment

Choose a reason for hiding this comment

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

Storybook works like a charm thanks !
But when I'm running yarn start I have a few errors on compile :

  • First with a onClick props to change to OnClick, which is quite straightforward.
  • The second is about typings (see screenshot or run yarn start to reproduce). Can you resolve before I merge ? Thanks

Capture d’écran 2019-11-15 à 13 10 25

@caiokf
Copy link
Contributor Author

caiokf commented Nov 17, 2019

@BenGamma Yes, will fix it. But question about the OnClick: is there a reason why it is capitalized? it comes from this commit. Can I make it lowercase everywhere?

Storybook works like a charm thanks !
But when I'm running yarn start I have a few errors on compile :

  • First with a onClick props to change to OnClick, which is quite straightforward.
  • The second is about typings (see screenshot or run yarn start to reproduce). Can you resolve before I merge ? Thanks
Capture d’écran 2019-11-15 à 13 10 25

@BenGamma
Copy link
Contributor

@BenGamma Yes, will fix it. But question about the OnClick: is there a reason why it is capitalized? it comes from this commit. Can I make it lowercase everywhere?

Storybook works like a charm thanks !
But when I'm running yarn start I have a few errors on compile :

  • First with a onClick props to change to OnClick, which is quite straightforward.
  • The second is about typings (see screenshot or run yarn start to reproduce). Can you resolve before I merge ? Thanks
Capture d’écran 2019-11-15 à 13 10 25

Yes you can lowercase it. It's legacy code, probably a mistake by the time 😇

@BenGamma BenGamma merged commit da4da44 into master Nov 17, 2019
@BenGamma BenGamma deleted the storybook-components branch November 17, 2019 16:45
@BenGamma BenGamma mentioned this pull request Nov 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants