feat: added network notice banner as a resuable component #112
Conversation
| @@ -25,8 +25,8 @@ | |||
| "prepack": "yarn build" | |||
| }, | |||
| "dependencies": { | |||
| "@agoric/rpc": "^0.9.0", | |||
| "@agoric/web-components": "^0.15.0", | |||
| "@agoric/rpc": "0.9.1-dev-02a116c.0+02a116c", | |||
There was a problem hiding this comment.
We want to keep these versions the same as in the local packages so they build from the workspace.
| @@ -0,0 +1,90 @@ | |||
| import { FiX } from 'react-icons/fi'; | |||
| import { GrAnnounce } from 'react-icons/gr'; | |||
| import { motion, AnimatePresence } from 'framer-motion'; | |||
There was a problem hiding this comment.
I'm unsure about importing framer-motion into this library, the bundle size is already really large. Maybe we can start without the collapse-on-hide animation, or just do it with regular CSS.
| }, | ||
| }; | ||
|
|
||
| const networkConfigAtom = atomWithStorage( |
There was a problem hiding this comment.
I think we should just have the component take the loaded network config JSON as an argument instead of hardcoding and fetching the network configs here. I have some ideas for handling network-configs more holistically in AgoricProvider, but I'm still not convinced we want the react components to deal with network configs.
|
In reference to Agoric/dapp-psm#94, I'm having trouble testing with yarn link as well (probably since we updated to yarn v4). I'm eager to put an example dapp in this package (#86) so we can test new components easier. |
Deploying ui-kit with
|
| Latest commit: |
ab2cc72
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://24c15ad2.ui-kit-dwm.pages.dev |
| Branch Preview URL: | https://feat-notice-banner.ui-kit-dwm.pages.dev |
yarn link was also troublesome for me. i've added instructions in the description on how to test these changes |
No issue exists previously. created one and linked it in the PR |
There was a problem hiding this comment.
No issue exists previously. created one and linked it in the PR
I see, I was wondering to see the motivation for creating this component. I think importing react-components into our inter dapps to use this might be heavy handed for now, but I have no qualms about providing a component like this in general, and it's a good exercise.
run yarn build in the ui-kit repo to create the dist directory
Heads up, we want to run yarn prepack from the top-level of ui-kit normally. This will build rpc, web-components, and react-components in order since they depend on eachother, in case you make changes to more than just react-components.
Also, you can remove the "WIP" from the PR now that it's out of draft.
| @@ -33,7 +33,8 @@ | |||
| "@leapwallet/elements": "0.12.1", | |||
| "chain-registry": "1.28.0", | |||
| "react": "18.2.0", | |||
| "react-dom": "18.2.0" | |||
| "react-dom": "18.2.0", | |||
| "react-icons": "^5.2.1" | |||
There was a problem hiding this comment.
Can we use something similar from https://heroicons.com/ and copy them into packages/react-components/src/lib/icons? Just trying to avoid adding dependencies and bundle size where possible. Chain-registry and Leap Elements already push the bundle size of this lib over 3MB which really pushes it, I'd like to figure out how to trim this down at some point.
| export const NoticeBanner = ({ | ||
| notices, | ||
| }: { | ||
| notices: Array<NetworkNotice>; |
There was a problem hiding this comment.
Let's export the props as "NoticeBannerProps" like https://github.com/Agoric/ui-kit/blob/main/packages/react-components/src/lib/components/AmountInput.tsx#L9. I do this so that they're accessible from the typedocs like https://9418f1f2.ui-kit-dwm.pages.dev/types/_agoric_react_components.AmountInputProps
| @@ -0,0 +1,22 @@ | |||
| export type NetworkNotice = { | |||
There was a problem hiding this comment.
Let's export these from packages/react-components/src/lib/utils/index.ts as well so they're accessible to consumers and in the typedocs
frazarshad
changed the title
frazarshad
force-pushed
the
feat/notice-banner
branch
from
ddb4e2a to
2de60fb
Compare
frazarshad
force-pushed
the
feat/notice-banner
branch
from
2de60fb to
ab2cc72
Compare
closes: #114
Description
This PR adds
NetworkBanner(originally implemented in this PR) as a reusable component for dapps. The purpose of this component is to query the/network-configendpoint to find any notices and display them.The use of this component dapp-econ-gov can be seen in this PR Agoric/dapp-econ-gov#104
How to test
dapp-econ-govandui-kitrepos and checkout to the `` andfeat/notice-bannerbranch respectivelyyarn buildin theui-kitrepo to create thedistdirectoryyarn add /full/path/to/ui-kit/packages/react-componentsindapp-econ-govto install it as a dependancy.Security Considerations
Scaling Considerations
Documentation Considerations
Testing Considerations