Skip to content

Conversation

@goldenapples
Copy link
Contributor

On registering UI for a shortcode, checks that any attributes of type "select" have a value set.

If not, sets their value to the key of the first option, as this is the behavior that will be expected by the user who sees that the first element in the select list appears to be selected on the form.

See #323.

On registering UI for shortcode, checks that any attributes of type
"select" have a value set. If not, sets their value to the key of the
first option, as this is the behavior that will be expected by the user
who sees that the first element in the select list appears to be
selected on the form.
@goldenapples
Copy link
Contributor Author

I'm really not sure where this logic should live. I tried it in a couple of different places: in the ShortcodeAttribute model and in the editAttributeField view.

Putting this logic in the view would make the most sense to me, as that's what it really affects, but the logic involved there was ugly, since the attribute inherits defaults which could be falsy.

An initialize function in the attribute model could work, but this was this simplest approach I could think of. As a matter of fact, I kind of think some more solid validation of shortcode attributes probably should be done in PHP before sending the shortcode attrs to javascript.

I'd like to hear thoughts on this if anyone has ideas.

@danielbachhuber
Copy link
Contributor

@sanchothefat Any opinions?

@danielbachhuber danielbachhuber added this to the v0.4.0 milestone Jun 1, 2015
@mattheu
Copy link
Contributor

mattheu commented Jun 8, 2015

Hmm... i'm not sure this is the right place for this. Although it is nice and simple.

I think this should probably go in the view, or possibly the attribute model.

@goldenapples
Copy link
Contributor Author

I think this should probably go in the view, or possibly the attribute model.

This was also made moot by #336, which now triggers the view's updateValue() method on render, setting the model to the initial state of the view. So this pull can be closed.

@danielbachhuber danielbachhuber deleted the 323-require-selects-to-have-value-set branch June 8, 2015 16:27
@danielbachhuber danielbachhuber removed this from the v0.4.0 milestone Jun 8, 2015
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.

4 participants