From a44bc5bcf3ff920c43b58d835d3c25b5185afd5c Mon Sep 17 00:00:00 2001 From: Garion Herman <garion@silverstripe.com> Date: Mon, 7 Oct 2019 10:57:31 +1300 Subject: [PATCH 1/4] NEW Add support for Tip UI in TextField See TextField documentation in silverstripe/admin Pattern Library --- src/Forms/TextField.php | 75 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/src/Forms/TextField.php b/src/Forms/TextField.php index 90f3d0259..8f4923ebd 100644 --- a/src/Forms/TextField.php +++ b/src/Forms/TextField.php @@ -16,6 +16,31 @@ class TextField extends FormField protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_TEXT; + /** + * @var bool Whether the Tip UI should be rendered + */ + protected $tipEnabled = false; + + /** + * @var string The contents of the Tip UI + */ + protected $tipMessage = ''; + + /** + * @var bool Whether the Tip should open immediately + */ + protected $tipOpenByDefault = false; + + /** + * @var string The icon that should be used on the Tip button + */ + protected $tipIcon = 'lamp'; + + /** + * @var string The Bootstrap color that the icon should be rendered in (e.g. warning, danger, success) + */ + protected $tipIconColor = 'muted'; + /** * Returns an input field. * @@ -59,6 +84,46 @@ class TextField extends FormField return $this->maxLength; } + /** + * Enables the Tip UI, which shows a popover on the right side of the field + * to place additional context or explanation of the field's purpose in. + * Currently only supported in React-based TextFields. + * + * @param string $message + * @param boolean $openByDefault Whether the Tip should open immediately + * @param string $icon An icon from the SilverStripe icon font + * @param null $iconColor A text colour defined by Bootstrap (e.g. warning, danger, success) + * @return $this + */ + public function enableTip($message, $openByDefault = false, $icon = null, $iconColor = null) + { + $this->tipEnabled = true; + $this->tipMessage = $message; + $this->tipOpenByDefault = $openByDefault; + + if ($icon) { + $this->tipIcon = $icon; + } + + if ($iconColor) { + $this->tipIconColor = $iconColor; + } + + return $this; + } + + /** + * Disables the Tip UI. The previous configuration is retained. + * + * @return $this + */ + public function disableTip() + { + $this->tipEnabled = false; + + return $this; + } + /** * @return array */ @@ -83,6 +148,16 @@ class TextField extends FormField { $data = parent::getSchemaDataDefaults(); $data['data']['maxlength'] = $this->getMaxLength(); + + if ($this->tipEnabled) { + $data['tip'] = [ + 'icon' => $this->tipIcon, + 'iconColor' => $this->tipIconColor, + 'content' => $this->tipMessage, + 'autoOpen' => $this->tipOpenByDefault, + ]; + } + return $data; } From efc7ba95200f761831477290cfde58011a2ec537 Mon Sep 17 00:00:00 2001 From: Garion Herman <garion@silverstripe.com> Date: Fri, 11 Oct 2019 15:04:56 +1300 Subject: [PATCH 2/4] NEW Tweak TextField Tip API to match changes to component --- src/Forms/TextField.php | 28 ++++++++++------------------ 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/src/Forms/TextField.php b/src/Forms/TextField.php index 8f4923ebd..c3a920c9e 100644 --- a/src/Forms/TextField.php +++ b/src/Forms/TextField.php @@ -27,20 +27,15 @@ class TextField extends FormField protected $tipMessage = ''; /** - * @var bool Whether the Tip should open immediately + * @var string How important the tip is (normal or high). Informs the color and description. */ - protected $tipOpenByDefault = false; + protected $tipImportance = 'normal'; /** * @var string The icon that should be used on the Tip button */ protected $tipIcon = 'lamp'; - /** - * @var string The Bootstrap color that the icon should be rendered in (e.g. warning, danger, success) - */ - protected $tipIconColor = 'muted'; - /** * Returns an input field. * @@ -90,25 +85,23 @@ class TextField extends FormField * Currently only supported in React-based TextFields. * * @param string $message - * @param boolean $openByDefault Whether the Tip should open immediately + * @param null|string $importance How important the tip is (normal or high); Informs the color and description * @param string $icon An icon from the SilverStripe icon font - * @param null $iconColor A text colour defined by Bootstrap (e.g. warning, danger, success) * @return $this */ - public function enableTip($message, $openByDefault = false, $icon = null, $iconColor = null) + public function enableTip($message, $importance = null, $icon = null) { $this->tipEnabled = true; $this->tipMessage = $message; - $this->tipOpenByDefault = $openByDefault; + + if ($importance) { + $this->tipImportance = $importance; + } if ($icon) { $this->tipIcon = $icon; } - if ($iconColor) { - $this->tipIconColor = $iconColor; - } - return $this; } @@ -151,10 +144,9 @@ class TextField extends FormField if ($this->tipEnabled) { $data['tip'] = [ - 'icon' => $this->tipIcon, - 'iconColor' => $this->tipIconColor, 'content' => $this->tipMessage, - 'autoOpen' => $this->tipOpenByDefault, + 'importance' => $this->tipImportance, + 'icon' => $this->tipIcon, ]; } From 195417b06120d1d7f2ec51b7ffff18259be1facc Mon Sep 17 00:00:00 2001 From: Garion Herman <garion@silverstripe.com> Date: Tue, 22 Oct 2019 17:04:58 +1300 Subject: [PATCH 3/4] NEW Extract Tip from TextField, add test coverage --- src/Forms/TextField.php | 60 +++----------- src/Forms/Tip.php | 126 ++++++++++++++++++++++++++++++ tests/php/Forms/TextFieldTest.php | 13 +++ tests/php/Forms/TipTest.php | 76 ++++++++++++++++++ 4 files changed, 228 insertions(+), 47 deletions(-) create mode 100644 src/Forms/Tip.php create mode 100644 tests/php/Forms/TipTest.php diff --git a/src/Forms/TextField.php b/src/Forms/TextField.php index c3a920c9e..2f87da55f 100644 --- a/src/Forms/TextField.php +++ b/src/Forms/TextField.php @@ -17,24 +17,9 @@ class TextField extends FormField protected $schemaDataType = FormField::SCHEMA_DATA_TYPE_TEXT; /** - * @var bool Whether the Tip UI should be rendered + * @var Tip|null A tip to render beside the input */ - protected $tipEnabled = false; - - /** - * @var string The contents of the Tip UI - */ - protected $tipMessage = ''; - - /** - * @var string How important the tip is (normal or high). Informs the color and description. - */ - protected $tipImportance = 'normal'; - - /** - * @var string The icon that should be used on the Tip button - */ - protected $tipIcon = 'lamp'; + protected $tip; /** * Returns an input field. @@ -80,39 +65,24 @@ class TextField extends FormField } /** - * Enables the Tip UI, which shows a popover on the right side of the field - * to place additional context or explanation of the field's purpose in. - * Currently only supported in React-based TextFields. - * - * @param string $message - * @param null|string $importance How important the tip is (normal or high); Informs the color and description - * @param string $icon An icon from the SilverStripe icon font - * @return $this + * @return Tip|null */ - public function enableTip($message, $importance = null, $icon = null) + public function getTip() { - $this->tipEnabled = true; - $this->tipMessage = $message; - - if ($importance) { - $this->tipImportance = $importance; - } - - if ($icon) { - $this->tipIcon = $icon; - } - - return $this; + return $this->tip; } /** - * Disables the Tip UI. The previous configuration is retained. + * Applies a Tip to the field, which shows a popover on the right side of + * the input to place additional context or explanation of the field's + * purpose in. Currently only supported in React-based forms. * + * @param Tip|null $tip The Tip to apply, or null to remove an existing one * @return $this */ - public function disableTip() + public function setTip(Tip $tip = null) { - $this->tipEnabled = false; + $this->tip = $tip; return $this; } @@ -142,12 +112,8 @@ class TextField extends FormField $data = parent::getSchemaDataDefaults(); $data['data']['maxlength'] = $this->getMaxLength(); - if ($this->tipEnabled) { - $data['tip'] = [ - 'content' => $this->tipMessage, - 'importance' => $this->tipImportance, - 'icon' => $this->tipIcon, - ]; + if ($this->tip instanceof Tip) { + $data['tip'] = $this->tip->getTipSchema(); } return $data; diff --git a/src/Forms/Tip.php b/src/Forms/Tip.php new file mode 100644 index 000000000..9ad86220e --- /dev/null +++ b/src/Forms/Tip.php @@ -0,0 +1,126 @@ +<?php + +namespace SilverStripe\Forms; + +use InvalidArgumentException; + +/** + * Represents a Tip which can be rendered alongside a form field in the front-end. + * See the Tip component in the silverstripe/admin module. + * + * @package SilverStripe\Forms + */ +class Tip +{ + /** + * These map to levels in the front-end Tip component + */ + const IMPORTANCE_LEVELS = [ + 'NORMAL' => 'normal', + 'HIGH' => 'high', + ]; + + const DEFAULT_ICON = 'lamp'; + + const DEFAULT_IMPORTANCE_LEVEL = self::IMPORTANCE_LEVELS['NORMAL']; + + /** + * @var string The icon that should be used on the Tip button + */ + private $icon; + + /** + * @var string How important the tip is (normal or high). Informs the color and description. + */ + private $importance_level; + + /** + * @var string The contents of the Tip UI + */ + private $message; + + public function __construct( + $message, + $importance_level = self::DEFAULT_IMPORTANCE_LEVEL, + $icon = self::DEFAULT_ICON + ) + { + if (!in_array($importance_level, self::IMPORTANCE_LEVELS)) { + throw new InvalidArgumentException( + 'Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS' + ); + } + + $this->message = $message; + $this->icon = $icon; + $this->importance_level = $importance_level; + } + + /** + * Outputs props to be passed to the front-end Tip component. + * + * @return array + */ + public function getTipSchema() + { + return [ + 'content' => $this->message, + 'icon' => $this->icon, + 'importance' => $this->importance_level, + ]; + } + + /** + * @return string + */ + public function getImportanceLevel() + { + return $this->importance_level; + } + + /** + * @param string $importance_level + */ + public function setImportanceLevel($importance_level) + { + if (!in_array($importance_level, self::IMPORTANCE_LEVELS)) { + throw new InvalidArgumentException( + 'Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS' + ); + } + + $this->importance_level = $importance_level; + } + + /** + * @return string + */ + public function getIcon(): string + { + return $this->icon; + } + + /** + * @param string $icon + */ + public function setIcon($icon) + { + $this->icon = $icon; + } + + /** + * @return string + */ + public function getMessage() + { + return $this->message; + } + + /** + * @param string $message + */ + public function setMessage($message) + { + $this->message = $message; + } +} diff --git a/tests/php/Forms/TextFieldTest.php b/tests/php/Forms/TextFieldTest.php index 380dff5bf..77acc0461 100644 --- a/tests/php/Forms/TextFieldTest.php +++ b/tests/php/Forms/TextFieldTest.php @@ -5,6 +5,7 @@ namespace SilverStripe\Forms\Tests; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\TextField; use SilverStripe\Forms\RequiredFields; +use SilverStripe\Forms\Tip; class TextFieldTest extends SapphireTest { @@ -32,4 +33,16 @@ class TextFieldTest extends SapphireTest $result = $textField->validate(new RequiredFields()); $this->assertTrue($result); } + + /** + * Ensures that when a Tip is applied to the field, it outputs it in the schema + */ + public function testTipIsIncludedInSchema() + { + $textField = new TextField('TestField'); + $this->assertArrayNotHasKey('tip', $textField->getSchemaDataDefaults()); + + $textField->setTip(new Tip('TestTip')); + $this->assertArrayHasKey('tip', $textField->getSchemaDataDefaults()); + } } diff --git a/tests/php/Forms/TipTest.php b/tests/php/Forms/TipTest.php new file mode 100644 index 000000000..d579934e8 --- /dev/null +++ b/tests/php/Forms/TipTest.php @@ -0,0 +1,76 @@ +<?php + +namespace SilverStripe\Forms\Tests; + +use InvalidArgumentException; +use SilverStripe\Dev\SapphireTest; +use SilverStripe\Forms\Tip; + +class TipTest extends SapphireTest +{ + /** + * Ensure the correct defaults are output in the schema + */ + public function testGeneratesAccurateDefaultSchema() + { + $tip = new Tip('message'); + + $schema = $tip->getTipSchema(); + + $this->assertEquals( + [ + 'content' => 'message', + 'icon' => 'lamp', + 'importance' => 'normal', + ], + $schema + ); + } + + /** + * Ensure custom settings are output in the schema + */ + public function testGeneratesAccurateCustomSchema() + { + $tip = new Tip( + 'message', + Tip::IMPORTANCE_LEVELS['HIGH'], + 'page' + ); + + $schema = $tip->getTipSchema(); + + $this->assertEquals( + [ + 'content' => 'message', + 'icon' => 'page', + 'importance' => 'high', + ], + $schema + ); + } + + /** + * Ensure passing an invalid importance level to the constructor fails + * + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS + */ + public function testInvalidImportanceLevelInConstructorCausesException() + { + $tip = new Tip('message', 'arbitrary-importance'); + } + + /** + * Ensure setting an invalid importance level fails + * + * @expectedException InvalidArgumentException + * @expectedExceptionMessage Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS + */ + public function testInvalidImportanceLevelInSetterCausesException() + { + $tip = new Tip('message'); + + $tip->setImportanceLevel('arbitrary-importance'); + } +} From bed3f2b3c691d3b157f615f1c0b6137f0908a40d Mon Sep 17 00:00:00 2001 From: Garion Herman <garion@silverstripe.com> Date: Wed, 23 Oct 2019 10:37:43 +1300 Subject: [PATCH 4/4] NEW Add type declarations to Tip API, add TippableFieldInterface --- src/Forms/TextField.php | 12 ++--- src/Forms/Tip.php | 68 ++++++++++++++++------------ src/Forms/TippableFieldInterface.php | 13 ++++++ tests/php/Forms/TipTest.php | 6 +-- 4 files changed, 60 insertions(+), 39 deletions(-) create mode 100644 src/Forms/TippableFieldInterface.php diff --git a/src/Forms/TextField.php b/src/Forms/TextField.php index 2f87da55f..0e261f7d7 100644 --- a/src/Forms/TextField.php +++ b/src/Forms/TextField.php @@ -7,7 +7,7 @@ use SilverStripe\Dev\Deprecation; /** * Text input field. */ -class TextField extends FormField +class TextField extends FormField implements TippableFieldInterface { /** * @var int @@ -19,7 +19,7 @@ class TextField extends FormField /** * @var Tip|null A tip to render beside the input */ - protected $tip; + private $tip; /** * Returns an input field. @@ -67,7 +67,7 @@ class TextField extends FormField /** * @return Tip|null */ - public function getTip() + public function getTip(): ?Tip { return $this->tip; } @@ -80,7 +80,7 @@ class TextField extends FormField * @param Tip|null $tip The Tip to apply, or null to remove an existing one * @return $this */ - public function setTip(Tip $tip = null) + public function setTip(?Tip $tip = null): self { $this->tip = $tip; @@ -112,8 +112,8 @@ class TextField extends FormField $data = parent::getSchemaDataDefaults(); $data['data']['maxlength'] = $this->getMaxLength(); - if ($this->tip instanceof Tip) { - $data['tip'] = $this->tip->getTipSchema(); + if ($this->getTip() instanceof Tip) { + $data['tip'] = $this->getTip()->getTipSchema(); } return $data; diff --git a/src/Forms/Tip.php b/src/Forms/Tip.php index 9ad86220e..508e3451c 100644 --- a/src/Forms/Tip.php +++ b/src/Forms/Tip.php @@ -1,4 +1,5 @@ <?php +declare(strict_types=1); namespace SilverStripe\Forms; @@ -7,22 +8,20 @@ use InvalidArgumentException; /** * Represents a Tip which can be rendered alongside a form field in the front-end. * See the Tip component in the silverstripe/admin module. - * - * @package SilverStripe\Forms */ class Tip { /** * These map to levels in the front-end Tip component */ - const IMPORTANCE_LEVELS = [ + public const IMPORTANCE_LEVELS = [ 'NORMAL' => 'normal', 'HIGH' => 'high', ]; - const DEFAULT_ICON = 'lamp'; + private const DEFAULT_ICON = 'lamp'; - const DEFAULT_IMPORTANCE_LEVEL = self::IMPORTANCE_LEVELS['NORMAL']; + private const DEFAULT_IMPORTANCE_LEVEL = self::IMPORTANCE_LEVELS['NORMAL']; /** * @var string The icon that should be used on the Tip button @@ -35,25 +34,24 @@ class Tip private $importance_level; /** - * @var string The contents of the Tip UI + * @var string The message to display in the tip */ private $message; + /** + * @param string $message The message to display in the tip + * @param string $importance_level How important the tip is (normal or high). Informs the color and description. + * @param string $icon The icon that should be used on the Tip button + * @throws InvalidArgumentException + */ public function __construct( - $message, - $importance_level = self::DEFAULT_IMPORTANCE_LEVEL, - $icon = self::DEFAULT_ICON - ) - { - if (!in_array($importance_level, self::IMPORTANCE_LEVELS)) { - throw new InvalidArgumentException( - 'Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS' - ); - } - - $this->message = $message; - $this->icon = $icon; - $this->importance_level = $importance_level; + string $message, + string $importance_level = self::DEFAULT_IMPORTANCE_LEVEL, + string $icon = self::DEFAULT_ICON + ) { + $this->setMessage($message); + $this->setIcon($icon); + $this->setImportanceLevel($importance_level); } /** @@ -61,35 +59,39 @@ class Tip * * @return array */ - public function getTipSchema() + public function getTipSchema(): array { return [ - 'content' => $this->message, - 'icon' => $this->icon, - 'importance' => $this->importance_level, + 'content' => $this->getMessage(), + 'icon' => $this->getIcon(), + 'importance' => $this->getImportanceLevel(), ]; } /** * @return string */ - public function getImportanceLevel() + public function getImportanceLevel(): string { return $this->importance_level; } /** * @param string $importance_level + * @return Tip + * @throws InvalidArgumentException */ - public function setImportanceLevel($importance_level) + public function setImportanceLevel(string $importance_level): self { if (!in_array($importance_level, self::IMPORTANCE_LEVELS)) { throw new InvalidArgumentException( - 'Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS' + 'Provided importance level must be defined in Tip::IMPORTANCE_LEVELS' ); } $this->importance_level = $importance_level; + + return $this; } /** @@ -102,25 +104,31 @@ class Tip /** * @param string $icon + * @return Tip */ - public function setIcon($icon) + public function setIcon(string $icon): self { $this->icon = $icon; + + return $this; } /** * @return string */ - public function getMessage() + public function getMessage(): string { return $this->message; } /** * @param string $message + * @return Tip */ - public function setMessage($message) + public function setMessage(string $message): self { $this->message = $message; + + return $this; } } diff --git a/src/Forms/TippableFieldInterface.php b/src/Forms/TippableFieldInterface.php new file mode 100644 index 000000000..cc1ecabe8 --- /dev/null +++ b/src/Forms/TippableFieldInterface.php @@ -0,0 +1,13 @@ +<?php + +namespace SilverStripe\Forms; + +/** + * Declares that a form field has the ability to accept and render Tips. + */ +interface TippableFieldInterface +{ + public function getTip(): ?Tip; + + public function setTip(Tip $tip); +} diff --git a/tests/php/Forms/TipTest.php b/tests/php/Forms/TipTest.php index d579934e8..02ea1a9ad 100644 --- a/tests/php/Forms/TipTest.php +++ b/tests/php/Forms/TipTest.php @@ -35,7 +35,7 @@ class TipTest extends SapphireTest $tip = new Tip( 'message', Tip::IMPORTANCE_LEVELS['HIGH'], - 'page' + 'page' ); $schema = $tip->getTipSchema(); @@ -54,7 +54,7 @@ class TipTest extends SapphireTest * Ensure passing an invalid importance level to the constructor fails * * @expectedException InvalidArgumentException - * @expectedExceptionMessage Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS + * @expectedExceptionMessage Provided importance level must be defined in Tip::IMPORTANCE_LEVELS */ public function testInvalidImportanceLevelInConstructorCausesException() { @@ -65,7 +65,7 @@ class TipTest extends SapphireTest * Ensure setting an invalid importance level fails * * @expectedException InvalidArgumentException - * @expectedExceptionMessage Provided $importance_level must be defined in Tip::IMPORTANCE_LEVELS + * @expectedExceptionMessage Provided importance level must be defined in Tip::IMPORTANCE_LEVELS */ public function testInvalidImportanceLevelInSetterCausesException() {