Skip to content
This repository has been archived by the owner on Feb 27, 2019. It is now read-only.

Add checkbox field type #19

Merged
merged 1 commit into from
Oct 31, 2018
Merged

Conversation

anehx
Copy link
Contributor

@anehx anehx commented Oct 25, 2018

Depends on #17

@anehx anehx requested review from kaldras and velrest October 25, 2018 10:39
@anehx anehx force-pushed the checkbox-field-type branch from b6eae2d to a5e11db Compare October 25, 2018 10:50
@anehx anehx requested a review from czosel October 29, 2018 10:03
@anehx anehx force-pushed the checkbox-field-type branch from a5e11db to b80bd48 Compare October 29, 2018 13:02
@anehx
Copy link
Contributor Author

anehx commented Oct 29, 2018

@czosel @kaldras Rebased and ready for review

listValue=(array "option-1")
)
)
}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't it be more readable to specify those nested objects in JS and pass them in as
field=field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a matter of taste I think. In this case it would maybe make sense, but I'd prefer to do it everywhere or nowhere in those tests. I think the best way would be to do it everywhere but in a seperate PR. Agree?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created an issue: #38

@anehx anehx merged commit 27e4cbd into projectcaluma:master Oct 31, 2018
@anehx anehx deleted the checkbox-field-type branch October 31, 2018 10:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants