add shadcn sheet and use on asset/collectible detail #2463
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
| "jest-canvas-mock": "2.5.2", | ||
| "jest-environment-jsdom": "29.7.0", | ||
| "js-yaml": "^4.1.1", | ||
| "js-yaml": "4.1.1", |
There was a problem hiding this comment.
I noticed 2 deps that should be pinned
| const [assetIcons, setAssetIcons] = useState(inputAssetIcons); | ||
| const networkDetails = useSelector(settingsNetworkDetailsSelector); | ||
| const [hasIconFetchRetried, setHasIconFetchRetried] = useState(false); | ||
| const [selectedAsset, setSelectedAsset] = useState<string>(""); |
There was a problem hiding this comment.
we can now move the selectedAsset state into this component rather than passing it in, which makes our code a little cleaner 🎉
| <span className="asset-code">{code}</span> | ||
| <div className="asset-native-amount" data-testid="asset-amount"> | ||
| {formatAmount(amountVal)} | ||
| <Sheet open={selectedAsset === canonicalAsset} key={canonicalAsset}> |
There was a problem hiding this comment.
Wrapping the existing component in <Sheet>
| selectedAsset={canonicalAsset} | ||
| handleClose={() => setSelectedAsset("")} | ||
| /> | ||
| </SheetContent> |
There was a problem hiding this comment.
And we then we wrap <AssetDetail> in <SheetContent>
| const CollectionsList = ({ collections }: { collections: Collection[] }) => { | ||
| const { t } = useTranslation(); | ||
| const [selectedCollectible, setSelectedCollectible] = | ||
| useState<SelectedCollectible | null>(null); |
There was a problem hiding this comment.
Similar change here - we can move the selected state closer to the actual detail component
| subentryCount, | ||
| tokenPrices, | ||
| assetIcons, | ||
| handleClose, |
There was a problem hiding this comment.
the real changes here are removing some of these passed params and replacing with the redux selectors. This helps clean up some prop drilling we would have to do
| module.exports = () => ({ | ||
| plugins: { | ||
| "@tailwindcss/postcss": {}, | ||
| autoprefixer: {}, |
There was a problem hiding this comment.
looks like using postcss to implement tailwind's animation lib is the cleanest way to integrate tailwind into our build system
| <img src={IconEllipsis} alt={t("asset options")} /> | ||
| </div> | ||
| {optionsOpen ? ( | ||
| <View> |
There was a problem hiding this comment.
wrapping the existing component in a View to make it conform to the correct height and width
| </div> | ||
| )} | ||
| {isNative && ( | ||
| <SlideupModal |
There was a problem hiding this comment.
We have a few instances of SlideupModal in the repo (a similar component to sheet). I've opened a new ticket to address this as I'd like to replace them all at once: #2467
Followup PR to #2451
This adds the
sheetcomponent from shadcn which we will use on asset/collectible detail views. This doesn't change any business logic, it's simply a refactor to uses the component when displaying the detail view. Because of that, I haven't added any new test; we expect our previous tests to provide coverage to make sure I haven't broken any functionalityScreen.Recording.2025-12-14.at.3.51.00.PM.mov