WIP: Style guide for Styleguidist UI - issue #897

[allison-zhao]

WIP for #897

[styleguidist-bot]

	Warnings
⚠️	Changes were made to package.json, but not to package-lock.json. If you’ve changed any dependencies (added, removed or updated any packages), please run npm install and commit changes in package-lock.json file. Make sure you’re using npm 5+.

Generated by 🚫 dangerJS

[sapegin]

Thanks for the pull request! I'll look into failing tests, I didn't expect that ;-£

[sapegin]

You can remove defaultExample: true — it will fix errors you see in the browsers console.

[allison-zhao]

@sapegin thanks! there was also a React error for getting the same key for an li element so i added Math.random() to the key, let me know if you prefer another way to do it :) Thanks for all the instructions!

Regarding the package-lock warning, I tried removing and npm install again but there's nothing new to commit (I guess it's because I just added a start script and it didn't affect any versioning in package-lock). any idea on what to do here? thanks!

[sapegin]

@sapegin thanks! there was also a React error for getting the same key for an li element so i added Math.random() to the key, let me know if you prefer another way to do it :)

Please don't use random keys in React, this is much worse than using indexes as keys (which can be acceptable in rare cases): you're telling React that you have a new object every render, so React has remove a previous element, create and mount a new one, instead of updating an existing one (in case of changed props) or not doing anything at all. This is never what you want ;-) Could you try filepath instead of name?

Regarding the package-lock warning, I tried removing and npm install again but there's nothing new to commit (I guess it's because I just added a start script and it didn't affect any versioning in package-lock).

Ignore that, we need to make the warning more clear ;-) You need to do that if you change any dependencies, which isn't the case here.

[allison-zhao]

@sapegin that's really helpful to know - always good to learn best practices in React. Just pushed the change, let me know if you need anything else!

[codecov-io]

Codecov Report

Merging #926 into master will decrease coverage by 0.05%.
The diff coverage is 100%.

Impacted Files	Coverage Δ
...omponents/ComponentsList/ComponentsListRenderer.js	100% <ø> (ø)	⬆️
scripts/schemas/config.js	87.5% <100%> (ø)	⬆️
styleguide.config.js	100% <100%> (ø)
test/apps/defaults/styleguide.config.js	100% <100%> (ø)
scripts/utils/getUserPackageJson.js	100% <100%> (ø)	⬆️
scripts/config.js	94.11% <100%> (-2.76%)	⬇️

[allison-zhao]

@sapegin let me know if there's anything else I can do to get this merged - thanks!

[sapegin]

Thank you, merged 🦄