-
Notifications
You must be signed in to change notification settings - Fork 8
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
Add position option for Toast #219
Conversation
src/components/Toast.tsx
Outdated
}, | ||
|
||
transitions: { | ||
from: { transform: "translateX(-10px)" }, |
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.
Repeating these will increase file size. Since these are strings, the change won't be negligible. We can't compromise on size for readability. So, extract these into separate objects and use them here.
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.
replacing repeating code with a dynamic function gives slightly longer code (see here)
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.
That link is giving 404
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.
What I asked was different.
const centerTransfrom = {
from: { transform: "translateX(-50%) translateY(-10px)" },
enter: { transform: "translateX(-50%) translateY(0)" },
leave: { transform: "translateX(-50%) translateY(-10px)" }
}
const customStyles = {
[ToastPosition.TOP]: {
style: {
top: 20,
left: "50%"
},
transitions: centerTransform
}}
now you can use centerTransform
at two places.
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.
the centerTransform
object won't be usable by ToastPosition.BOTTOM
because the translateY()
is negative but needs to be positive
also the tests are failing. |
fixed tests |
stories/toast.story.tsx
Outdated
Toast.show("hello", "success", { | ||
time: 1000, | ||
position: select( | ||
"bottom", |
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.
change this to position
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.
fixed in 5c997ea
Also, It's crashing for TOP position in storybook |
Run |
I guess tests are failing because the code generated by Typescript is not fully tree-shakeable. |
exporting
|
fixes #218