-
Notifications
You must be signed in to change notification settings - Fork 30
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loving this library!
A few suggestions/questions.
README.md
Outdated
``` | ||
|
||
And then include it wrapping your application. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about the wrapping as it's not wrapping it, but at the same App
level.
Co-authored-by: Fernando <fernando.greco@gmail.com>
Co-authored-by: Fernando <fernando.greco@gmail.com>
Co-authored-by: Fernando <fernando.greco@gmail.com>
Co-authored-by: Fernando <fernando.greco@gmail.com>
Can we add what version of material-ui is required ... looking in the package.json is annoying :P |
styled-components is also a peer dependency, right? |
README.md
Outdated
This library makes use of [material-ui](https://material-ui.com/) as a `peerDependency`, this means you must install it in your Safe App. Make sure to provide the same version as the one being used by the current version of this library. | ||
|
||
Once everything is installed, you have to instantiate a [ThemeProvider](https://styled-components.com/docs/api#themeprovider) from [styled-components](https://@gnosis.pm/safe-react-components/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why does styled-components
link to https://@gnosis.pm/safe-react-components/
|
||
```bash | ||
yarn add @gnosis.pm/safe-react-components |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should also include npm command
src: local('Averta'), local('Averta Bold'), | ||
url(${avertaFont}) format('woff2'), | ||
url(${avertaBoldFont}) format('woff'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include font-display swap in this example
https://developers.google.com/web/updates/2016/02/font-display
Changes pushed! thx for the review! |
Travis automatic deployment: |
No description provided.