Skip to content

Make sure that when we have a value, we also show the label #14

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

compojoom
Copy link
Collaborator

When a user loads a form in our application it's by default empty. However we have a "favorites" feature - the user clicks on previous values and the form is pre-populated with data. This works, but the label was missing as it's set in the constructor to 0 when we don't have a value.

In this pull request I'm comparing the next props and if it has a value I'm setting fadeAnim to true

})
}

super.componentWillReceiveProps(next)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this line needed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep. The parent component sets the internal state for value. We could either use this, or set the state ourself. But I thought it would be better to let the parent class manage what it is supposed to manage anyway.

@compojoom
Copy link
Collaborator Author

@alvaromb - want to merge this?

@alvaromb
Copy link
Collaborator

@compojoom scheduled for tomorrow! Sorry, busy as hell (as always :))

Want to join as a collaborator?

@compojoom
Copy link
Collaborator Author

Sure. I'm using this in our project, so happy to help where I can.

@compojoom
Copy link
Collaborator Author

@alvaromb turned out that when having a 0 value the component was not behaving properly and was not showing the label on top. fixed this in the last commit as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants