Skip to content
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

fix(vx-responsive): fix enhancer parent/screen size checks #621

Merged
merged 1 commit into from
Feb 4, 2020

Conversation

williaster
Copy link
Collaborator

🐛 Bug Fix

Fixes #619 where the TS re-write introduced a regression wherein the state is initialized with undefined, while the render method checks for nulls

@hshoff @kristw
cc @dson

@williaster williaster added this to the v0.0.194 milestone Feb 3, 2020
Copy link
Collaborator

@kristw kristw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking. Just ask whether 0 width or height should be rendered?

>
{parentWidth !== null && parentHeight !== null && (
<div style={CONTAINER_STYLES} ref={this.setRef}>
{parentWidth != null && parentHeight != null && (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to check for 0 as well?

@@ -54,7 +54,7 @@ export default function withScreenSize<BaseComponentProps extends WithScreenSize

render() {
const { screenWidth, screenHeight } = this.state;
if (!screenWidth && !screenHeight) return null;
if (screenWidth == null || screenHeight == null) return null;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to check for 0 as well?

@williaster
Copy link
Collaborator Author

williaster commented Feb 4, 2020

hmm 🤔 if it is 0 and a consumer wants to handle that case for some reason they won't be able to. we were previously inconsistent between the two HOCs, parentSize would render with 0 and screenSize would not (but only if both were 0).

I think it's okay for us to pass 0s and let the consumer handle that case for now.

@williaster
Copy link
Collaborator Author

Also noting that rendering with 0s mirrors the behavior of <ParentSize /> which does not do any check.

@williaster williaster merged commit 368b57b into master Feb 4, 2020
@williaster williaster deleted the chris--w-parent-size-fix-619 branch February 4, 2020 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

withParentSize no longer delays rendering until size is known
2 participants