search

AmpersandJS / ampersand-select-view

Select field for ampersand-form-views
MIT License
11 stars 26 forks source link

Hide label when opts.label is not set (fixes: #53) #62

Closed e2jk closed 8 years ago

cdaringe commented 8 years ago

This request removes the default configuration of a field label fallback. I know that its presence has caused some grief, and that's the last thing we want, but certainly some users use the fallback and are content with it. Would simply setting label: null fix your issue? the node would still be present, but empty, and likely not impacting the rendering of the DOM unless there was some wily nily CSS at play.

thoughts?

cdaringe commented 8 years ago

meh, ok, in favor of matching input view's behavior, i'm in support of it, however it will be a major bump.

+1 once test are confirmed passing

cdaringe commented 8 years ago

@e2jk, will you pull the latest upstream master into your branch? hopefully that resolves the testing issues :)

wraithgar commented 8 years ago

+1 yeah this will be a major. Could a test be added for this?

wraithgar commented 8 years ago

@e2jk I made this PR (https://github.com/AmpersandJS/ampersand-select-view/pull/63) w/ tests updated and it looks like there's still a bug here. The label ends up being the string 'undefined' somehow. Will need to get that test passing before we can merge (either in this PR or in the other one, doesn't matter. Once one works we'll merge and close the other)

wraithgar commented 8 years ago

Found it! Closing this in favor of the other PR which has your work in it already @e2jk plus the tweak to get tests passing. Thanks for pushing for this change it feels like the right one, all told.

e2jk commented 8 years ago

Great!