Changeset View
Changeset View
Standalone View
Standalone View
src/view/form/control/AphrontFormControl.php
| Show First 20 Lines • Show All 166 Lines • ▼ Show 20 Lines | final public function render() { | ||||
| } | } | ||||
| $input = phutil_tag( | $input = phutil_tag( | ||||
| 'div', | 'div', | ||||
| array('class' => 'aphront-form-input'), | array('class' => 'aphront-form-input'), | ||||
| $this->renderInput()); | $this->renderInput()); | ||||
| $error = null; | $error = null; | ||||
| if (strlen($this->getError())) { | if ($this->getError()) { | ||||
valerio.bozzolan: The problem with `if($error)` is that it discards errors message like `"0"` and Evan is sad. | |||||
| $error = $this->getError(); | $error = $this->getError(); | ||||
| if ($error === true) { | if ($error === true) { | ||||
| $error = phutil_tag( | $error = phutil_tag( | ||||
| 'span', | 'span', | ||||
| array('class' => 'aphront-form-error aphront-form-required'), | array('class' => 'aphront-form-error aphront-form-required'), | ||||
| pht('Required')); | pht('Required')); | ||||
| } else { | } else { | ||||
| $error = phutil_tag( | $error = phutil_tag( | ||||
| 'span', | 'span', | ||||
| array('class' => 'aphront-form-error'), | array('class' => 'aphront-form-error'), | ||||
| $error); | $error); | ||||
| } | } | ||||
| } | } | ||||
| if (strlen($this->getLabel())) { | if (phutil_nonempty_string($this->getLabel())) { | ||||
Done Inline Actions✅ The input value is probably safe to be assumed as string or NULL. Alien values will be reported and that is OK. I checked 30+ usages and the label is probably always a raw string containing HTML for example and I have not noticed objects with toString() methods. Average usage example: ->setLabel(pht('Subscribers'))valerio.bozzolan: ✅ The input value is probably safe to be assumed as `string` or NULL. Alien values will be… | |||||
| $label = phutil_tag( | $label = phutil_tag( | ||||
| 'label', | 'label', | ||||
| array( | array( | ||||
| 'class' => 'aphront-form-label', | 'class' => 'aphront-form-label', | ||||
| 'for' => $this->getID(), | 'for' => $this->getID(), | ||||
| ), | ), | ||||
| array( | array( | ||||
| $this->getLabel(), | $this->getLabel(), | ||||
| $error, | $error, | ||||
| )); | )); | ||||
| } else { | } else { | ||||
| $label = null; | $label = null; | ||||
| $custom_class .= ' aphront-form-control-nolabel'; | $custom_class .= ' aphront-form-control-nolabel'; | ||||
| } | } | ||||
| if (strlen($this->getCaption())) { | if (phutil_nonempty_string($this->getCaption())) { | ||||
Done Inline Actions✅ The input value is probably safe to be assumed as string or NULL. Alien values will be reported and that is OK. I checked 30+ usages and the caption is always a raw string containing HTML for example and I have not noticed objects with toString() methods. Average usage example: ->setCaption(
pht('Example: %s', phutil_tag('tt', array(), 'JIS-3, JIS-9'))))valerio.bozzolan: ✅ The input value is probably safe to be assumed as `string` or NULL. Alien values will be… | |||||
| $caption = phutil_tag( | $caption = phutil_tag( | ||||
| 'div', | 'div', | ||||
| array('class' => 'aphront-form-caption'), | array('class' => 'aphront-form-caption'), | ||||
| $this->getCaption()); | $this->getCaption()); | ||||
| } else { | } else { | ||||
| $caption = null; | $caption = null; | ||||
| } | } | ||||
| Show All 30 Lines | |||||
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under Apache 2.0 or other open source licenses. · CC BY-SA 4.0 · Apache 2.0
The problem with if($error) is that it discards errors message like "0" and Evan is sad.
Probably this is a good example for phutil_nonempty_scalar() since it accepts booleans and other stuff. But we need to merge this before:
D25117: phutil_nonempty_scalar(): don't throw when receiving a boolean scalar
@avivey what do you think about?