Changeset View
Changeset View
Standalone View
Standalone View
src/view/form/control/AphrontFormControl.php
Show First 20 Lines • Show All 50 Lines • ▼ Show 20 Lines | public function setLabel($label) { | ||||
$this->label = $label; | $this->label = $label; | ||||
return $this; | return $this; | ||||
} | } | ||||
public function getLabel() { | public function getLabel() { | ||||
return $this->label; | return $this->label; | ||||
} | } | ||||
/** | |||||
* Set the Caption | |||||
* The Caption shows a tip usually nearby the related input field. | |||||
* @param string|PhutilSafeHTML|null | |||||
* @return self | |||||
*/ | |||||
public function setCaption($caption) { | public function setCaption($caption) { | ||||
$this->caption = $caption; | $this->caption = $caption; | ||||
return $this; | return $this; | ||||
} | } | ||||
/** | |||||
* Get the Caption | |||||
* The Caption shows a tip usually nearby the related input field. | |||||
* @return string|PhutilSafeHTML|null | |||||
*/ | |||||
public function getCaption() { | public function getCaption() { | ||||
return $this->caption; | return $this->caption; | ||||
} | } | ||||
public function setError($error) { | public function setError($error) { | ||||
$this->error = $error; | $this->error = $error; | ||||
return $this; | return $this; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 126 Lines • ▼ Show 20 Lines | if (phutil_nonempty_string($this->getLabel())) { | ||||
$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 (phutil_nonempty_string($this->getCaption())) { | // The Caption can be stuff like PhutilSafeHTML and other objects that | ||||
valerio.bozzolan: ↑ This explodes in cases of a caption consisting in `PhutilSafeHTML`
I discovered that during… | |||||
// can be casted to a string. After this cast we have never null. | |||||
$has_caption = phutil_string_cast($this->getCaption()) !== ''; | |||||
if ($has_caption) { | |||||
$caption = phutil_tag( | $caption = phutil_tag( | ||||
'div', | 'div', | ||||
array('class' => 'aphront-form-caption'), | array('class' => 'aphront-form-caption'), | ||||
Not Done Inline Actionsthis whole section might be clearer like this: $caption = $this->getCaption(); $has_caption = $caption || (phutil_string_cast($caption) !== ''); or some other variation on $has_caption rather then content that isn't used as content. avivey: this whole section might be clearer like this:
```lang=php
$caption = $this->getCaption()… | |||||
Done Inline ActionsOK I get your point Premising that probably the suggested code will not work for objects since an object short-circuit as truly even if it returns an empty string. So it would be necessary a cast call in any case. Premising that doing a cast seems very good to me since that was what PHP was doing under the hood with strlen stuff. valerio.bozzolan: OK I get your point
Premising that probably the suggested code will not work for objects since… | |||||
$this->getCaption()); | $this->getCaption()); | ||||
Done Inline ActionsReason for that comment: If phutil_tag receives a string, it escapes that. But we don't want to see <tt>Stuff</tt> at video. valerio.bozzolan: Reason for that comment:
If `phutil_tag` receives a string, it escapes that. But we don't want… | |||||
} else { | } else { | ||||
$caption = null; | $caption = null; | ||||
} | } | ||||
$classes = array(); | $classes = array(); | ||||
$classes[] = 'aphront-form-control'; | $classes[] = 'aphront-form-control'; | ||||
$classes[] = 'grouped'; | $classes[] = 'grouped'; | ||||
$classes[] = $custom_class; | $classes[] = $custom_class; | ||||
Show All 26 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
↑ This explodes in cases of a caption consisting in PhutilSafeHTML
I discovered that during heavy testing of other random pages outside the reasonable test plan