-
Notifications
You must be signed in to change notification settings - Fork 5.3k
feat: add AssetDownload component stories for Storybook #16260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
Hey @MahendraBishnoi29, thanks for this! Getting this error on each of these when viewing locally: Path must be a string. Received null
Small house-keeping note as well, if you use the words "Closes" or "Fixes" before the issue number in your PR description it will properly link it to that issue, so the issue is auto-closed when the PR is merged. And bonus, if it's used with a bullet-point, GitHub will render it nicely with the issue title and status as well:
## Related Issue
- Closes #13045
I've gone ahead and updated this, but just noting for your sake =)
…AssetDownload component
Fixed it, please check and let me know if any changes needed. @wackerow |
|
@MahendraBishnoi29 hm, I'm still getting this same error.. are you able to load this up successfully on your end with |
…me in Link component
Seems like the |
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.
Thanks @MahendraBishnoi29! I pushed a couple commits to try and clean a few things up
- To get the intl strings working, we had to add the
page-assetsnamespace to the .storybook/next-intl.ts config - Updated the location of this story to nest inside "Molecules / Display Content / AssetDownload"
- Switched the initial example to use the hero image
- I reverted the changes to ui/Link and AssetDownload/index.tsx to minimize new changes to other components
- To fix the
undefinedpathname issue, added a small patch instead to thegetRelativePathutil to makerelativePathoptional, falling back to an empty string.
Letting the preview build now, but approving in the meantime assuming it builds without issues. The story is working properly for me locally.. thanks again!
|
@all-contributors please add @MahendraBishnoi29 for test |


Description
The two stories I created,
WithArtistandBrandAsset, are designed to reflect the primary ways theAssetDownloadcomponent is used on our current assets page.These stories test the component's rendering in the main contexts you're using it in.
Related Issue