accordproject / cicero-ui

A library of React components for Accord Project templates
Apache License 2.0
29 stars 46 forks source link

feat(Storybook): extend storybook for developer focused demo - #315 #382

Closed 98lenvi closed 4 years ago

98lenvi commented 4 years ago

Issue #315 #376

Complete support added for Cicero-UI components in Storybook.

Preview available here

Changes

Previously added, now removed-

Flags

Need to add the following (DONE)

Related Issues

98lenvi commented 4 years ago

@irmerk, I hope it's complete now. Can you please go through it and let me know if anything needs to be added / modified

98lenvi commented 4 years ago

Added a few comments. Also, is there a way to see the netlify build of storybook for this PR?

@DianaLease I will be adding a netlify.toml file for that in a commit.

irmerk commented 4 years ago

This is super exciting @98lenvi, great work. Looking into the code now.

irmerk commented 4 years ago

In the Notes tab for most of these, we'll want to make sure to format some things to make it consistent with the .md format here in the GitHub repo. Example is the main Demo which has the README.md from the main repo here, and the table for Ecosystem > Core libraries is a bit off.

dselman commented 4 years ago

In the Notes tab for most of these, we'll want to make sure to format some things to make it consistent with the .md format here in the GitHub repo. Example is the main Demo which has the README.md from the main repo here, and the table for Ecosystem > Core libraries is a bit off.

Let's try to avoid duplication as much as possible to keep things maintainable. E.g. link to the existing README.md.

98lenvi commented 4 years ago

In the Notes tab for most of these, we'll want to make sure to format some things to make it consistent with the .md format here in the GitHub repo. Example is the main Demo which has the README.md from the main repo here, and the table for Ecosystem > Core libraries is a bit off.

I will look into that, can't understand why the formatting looks off. @irmerk

Let's try to avoid duplication as much as possible to keep things maintainable. E.g. link to the existing README.md.

Done in the latest commit. @dselman

irmerk commented 4 years ago

I'm thinking we get this in and start iterating on it to stop blocking this PR. Just need to make sure local development of ContractEditor still works as we expect.

98lenvi commented 4 years ago

@irmerk Sure! I just need to work on one more commit to add knobs to ContractEditor story ( I missed the lockText and readOnly props)

98lenvi commented 4 years ago

@irmerk, I made the changes, i.e. add readOnly knob. For some reason, lockText doesn't work via a knob. Even if I declare it as a state variable and change its value, there is no effect on the ContractEditor.

DianaLease commented 4 years ago

@irmerk, I made the changes, i.e. add readOnly knob. For some reason, lockText doesn't work via a knob. Even if I declare it as a state variable and change its value, there is no effect on the ContractEditor.

This sounds similar to an issue we had with the old version of slate where prop changes wouldn't re-render the editor. It was working for me with the new slate when we had a toggle switch though that updated the value of lockText. Let me try again locally with a toggle and reconfirm.

EDIT: @98lenvi looks like this is a separate issue. I'll look into it!

irmerk commented 4 years ago

Confirming this works for local development (npm run storybook instead of npm run start)