-
Notifications
You must be signed in to change notification settings - Fork 62
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(ui): non full-windowed support #55
base: master
Are you sure you want to change the base?
Conversation
* Useful if using a navbar, or any other component that obscurs part of the screen.
@VincentCATILLON Bumping in case you have not seen this yet and are interested :) |
@VincentCATILLON Are you still maintaining this repository, or is there a better place to merge this? |
Sorry @bertrand-caron about the delay, I'll make a review asap |
|------------------|---------------------------------|--------------------------------------------|----------|--------------------------| | ||
| count | number | items count to display | required | | | ||
| origin | {x: number, y: number} | animation position origin | required | | | ||
| window | {width: number, height: number} | width and height of the animation "window" | | Dimensions.get('window') | |
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.
| window | {width: number, height: number} | width and height of the animation "window" | | Dimensions.get('window') | | |
| dimensions | {width: number, height: number} | width and height of the animation "window" | | Dimensions.get('window') | |
looks more explicit than window
@@ -6,6 +6,10 @@ export interface ExplosionProps { | |||
x: number; | |||
y: number | |||
}; | |||
window?: { |
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.
window?: { | |
dimensions?: { |
@@ -14,6 +14,10 @@ type Props = {| | |||
x: number, | |||
y: number | |||
}, | |||
window?: { |
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.
window?: { | |
dimensions?: { |
@@ -178,9 +182,9 @@ class Explosion extends React.PureComponent<Props, State> { | |||
}; | |||
|
|||
render() { | |||
const { origin, fadeOut } = this.props; | |||
const { origin, fadeOut, window } = this.props; |
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.
const { origin, fadeOut, window } = this.props; | |
const { origin, fadeOut, dimensions = Dimensions.get('window') } = this.props; |
const { items } = this.state; | ||
const { height, width } = Dimensions.get('window'); | ||
const { height, width } = window ?? Dimensions.get('window'); |
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.
const { height, width } = window ?? Dimensions.get('window'); | |
const { height, width } = dimensions; |
Thanks for your nice contribution I think the prop should be more explicit ( |
Can this get merged @VincentCATILLON would be helpful |
Description
Add support for non-fullscreen confetti. Useful if using a navbar, or any other component that obscures part of the screen. behaviour is backwards compatible (defaults to
Dimensions.get('window')
).Result
N/A
Related issues
N/A