organization modal #26
Conversation
|
|
||
| const organizationsNotEmpty = Object.keys(organizations).length > 0 | ||
|
|
||
| const cardProps = { |
There was a problem hiding this comment.
I think this is okay for now, but it would be nice to remove any unnecessary variables if possible. If we could just add these as props that would save us another variable here. But i think this pattern exists elsewhere in the app, so it might be easier to just make one sweeping change. What are your thoughts?
There was a problem hiding this comment.
if only "Object.keys(organizations).length" could be expressed in more concise way... How about replacing it with _.isEmpty() from lodash? Remove extra variable, replace the length test with isEmpty?
There was a problem hiding this comment.
I was referring to cardProps, not organizationNotEmpty. But i don't think we should add another dependency just to make that check. The check you're making is fine and can still be written in one line anyways.
There was a problem hiding this comment.
Hey, this is really close. You just need to update the forms in state to add room for organizations. Will wait for @chadoh to review as well before merging.
This is for just displaying the modal. Not very functional by now, it's mainly missing storing the address somewhere: whihc means validation is also complaining. It's ready for integration though. Also, Organizations Card uses Education tiles until it has its own tiles (created in another ticket).