Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: better storybook stories for the notification pages #27861
feat: better storybook stories for the notification pages #27861
Changes from 24 commits
5019bf2
910651a
1b492ff
59d7b22
cf66689
e7bb1b2
6f24104
2322c20
36288c3
7f9b8c6
4096501
d44e45b
14cacbc
55222f0
5c3ba50
be54666
be62ddb
b95a324
0f82b1a
e016bdb
312be96
c8ec8d1
843ef49
15c45d9
1162ecf
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
This sub-optimal solution was previously introduced to handle a dedicated header for RPC calls to the Infura endpoint. In this PR, the use of this call is managed without the header in the case of a Storybook build
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.
Storybook registers certain global variables that you can check for during runtime. For instance, Storybook adds a window.STORYBOOK_ADDONS_CHANNEL property when it runs.
You can use this to conditionally detect Storybook like so:
const isStorybook = typeof window !== 'undefined' && !!window.__STORYBOOK_ADDONS_CHANNEL__;
This way, you won’t need the STORYBOOK_ENV variable; instead, you’re checking for a runtime indication that you’re in Storybook.
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.
thx @andreahaku . I verified that Storybook indeed sets its own env, so I can check
process.env.STORYBOOK
without having to introduce a new ENVThere 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.
what about reducing redundancy and something like:
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.
Nice cleanup, lets either add this now or in the next subsequent PR.
For context, the number of notification types we will support is going to grow substantially over time. So being able to create storybooks for each one easily (without much effort) will be important.
aave_v3_health_factor
;ens_expiration
;lido_staking_rewards
;notional_loan_expiration
;rocketpool_staking_rewards
;spark_fi_health_factor
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, Andrea! Actually, I was thinking of optimizing these stories in a new PR I’m working on because we’re introducing new stories (as @Prithpal-Sooriya mentioned). But you’re right; better to do it right away!