From fffc1e51d0d8bfed5f75cf3f245fa50f09c3c8be Mon Sep 17 00:00:00 2001 From: Hafsteinn Baldvinsson Date: Thu, 15 Mar 2012 17:10:19 -0700 Subject: [PATCH] Inset view controller for inset elements of forms. Summary: T937 suggests 'inset' could have its own view controller. It has the following methods: - setTitle for title - setRightbutton if you have to place something (preferably a button) on the right side of the form - setDescription if you want to describe what it does - setContent for the main content - addDivAttributes REALLY not sure about this one but it had to be included because of a single controller (see owners/controller/edit/PhabricatorOwnersEditController.php:238) - appendChild works as usual if your form is complex but you still want to remove ->appendChild('
appendChild('
'); It might be an overkill so maybe some could be dropped: - addDivAttributes() and just rewrite how PhabricatorOwnersEditController.php works - setContent() and use appendChild for the main content? Test Plan: - Looked at the controllers in phabricator - Changed the controller - Opened the page in another tab - If something didnd't look the same I fixed it. Reviewers: epriestley Reviewed By: epriestley CC: aran, epriestley Differential Revision: https://secure.phabricator.com/D1926 --- src/__phutil_library_map__.php | 2 + .../controller/rule/HeraldRuleController.php | 67 +++++----- .../herald/controller/rule/__init__.php | 1 + .../batch/ManiphestBatchEditController.php | 16 +-- .../maniphest/controller/batch/__init__.php | 1 + .../edit/PhabricatorOwnersEditController.php | 31 ++--- .../owners/controller/edit/__init__.php | 1 + ...habricatorProjectProfileEditController.php | 19 +-- .../controller/profileedit/__init__.php | 1 + .../PhabricatorRepositoryEditController.php | 89 ++++++------- .../repository/controller/edit/__init__.php | 1 + src/view/form/inset/AphrontFormInsetView.php | 123 ++++++++++++++++++ src/view/form/inset/__init__.php | 14 ++ 13 files changed, 243 insertions(+), 123 deletions(-) create mode 100644 src/view/form/inset/AphrontFormInsetView.php create mode 100644 src/view/form/inset/__init__.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 412c7aa7ef..411c8c2f80 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -34,6 +34,7 @@ phutil_register_library_map(array( 'AphrontFormDividerControl' => 'view/form/control/divider', 'AphrontFormDragAndDropUploadControl' => 'view/form/control/draganddropupload', 'AphrontFormFileControl' => 'view/form/control/file', + 'AphrontFormInsetView' => 'view/form/inset', 'AphrontFormLayoutView' => 'view/form/layout', 'AphrontFormMarkupControl' => 'view/form/control/markup', 'AphrontFormPasswordControl' => 'view/form/control/password', @@ -935,6 +936,7 @@ phutil_register_library_map(array( 'AphrontFormDividerControl' => 'AphrontFormControl', 'AphrontFormDragAndDropUploadControl' => 'AphrontFormControl', 'AphrontFormFileControl' => 'AphrontFormControl', + 'AphrontFormInsetView' => 'AphrontView', 'AphrontFormLayoutView' => 'AphrontView', 'AphrontFormMarkupControl' => 'AphrontFormControl', 'AphrontFormPasswordControl' => 'AphrontFormControl', diff --git a/src/applications/herald/controller/rule/HeraldRuleController.php b/src/applications/herald/controller/rule/HeraldRuleController.php index 43fe41aa2e..d1659c9090 100644 --- a/src/applications/herald/controller/rule/HeraldRuleController.php +++ b/src/applications/herald/controller/rule/HeraldRuleController.php @@ -165,34 +165,31 @@ final class HeraldRuleController extends HeraldController { "This ${rule_type_name} rule triggers for " . "${content_type_name}.")) ->appendChild( - '

Conditions

'. - '
'. - '
'. - javelin_render_tag( - 'a', - array( - 'href' => '#', - 'class' => 'button green', - 'sigil' => 'create-condition', - 'mustcapture' => true, - ), - 'Create New Condition'). - '
'. - '

When '.$must_match_selector.' these conditions are met:

'. - '
'. - javelin_render_tag( + id(new AphrontFormInsetView()) + ->setTitle('Conditions') + ->setRightButton(javelin_render_tag( + 'a', + array( + 'href' => '#', + 'class' => 'button green', + 'sigil' => 'create-condition', + 'mustcapture' => true + ), + 'Create New Condition')) + ->setDescription( + 'When '.$must_match_selector . + ' these conditions are met:') + ->setContent(javelin_render_tag( 'table', array( 'sigil' => 'rule-conditions', - 'class' => 'herald-condition-table', + 'class' => 'herald-condition-table' ), - ''). - '
') + ''))) ->appendChild( - '

Action

'. - '
'. - '
'. - javelin_render_tag( + id(new AphrontFormInsetView()) + ->setTitle('Action') + ->setRightButton(javelin_render_tag( 'a', array( 'href' => '#', @@ -200,20 +197,16 @@ final class HeraldRuleController extends HeraldController { 'sigil' => 'create-action', 'mustcapture' => true, ), - 'Create New Action'). - '
'. - '

'. - 'Take these actions '.$repetition_selector.' this rule matches:'. - '

'. - '
'. - javelin_render_tag( - 'table', - array( - 'sigil' => 'rule-actions', - 'class' => 'herald-action-table', - ), - ''). - '
') + 'Create New Action')) + ->setDescription('Take these actions '.$repetition_selector. + ' this rule matches:') + ->setContent(javelin_render_tag( + 'table', + array( + 'sigil' => 'rule-actions', + 'class' => 'herald-action-table', + ), + ''))) ->appendChild( id(new AphrontFormSubmitControl()) ->setValue('Save Rule') diff --git a/src/applications/herald/controller/rule/__init__.php b/src/applications/herald/controller/rule/__init__.php index cc92ef1ec7..7167b40e78 100644 --- a/src/applications/herald/controller/rule/__init__.php +++ b/src/applications/herald/controller/rule/__init__.php @@ -28,6 +28,7 @@ phutil_require_module('phabricator', 'view/form/control/markup'); phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/error'); +phutil_require_module('phabricator', 'view/form/inset'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php b/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php index 57aa55005f..08e0ab6cd9 100644 --- a/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php +++ b/src/applications/maniphest/controller/batch/ManiphestBatchEditController.php @@ -104,10 +104,9 @@ final class ManiphestBatchEditController extends ManiphestController { $form->appendChild('

These tasks will be edited:

'); $form->appendChild($list); $form->appendChild( - '

Actions

'. - '
'. - '
'. - javelin_render_tag( + id(new AphrontFormInsetView()) + ->setTitle('Actions') + ->setRightButton(javelin_render_tag( 'a', array( 'href' => '#', @@ -115,17 +114,14 @@ final class ManiphestBatchEditController extends ManiphestController { 'sigil' => 'add-action', 'mustcapture' => true, ), - 'Add Another Action'). - '
'. - '
'. - javelin_render_tag( + 'Add Another Action')) + ->setContent(javelin_render_tag( 'table', array( 'sigil' => 'maniphest-batch-actions', 'class' => 'maniphest-batch-actions-table', ), - ''). - '
') + ''))) ->appendChild( id(new AphrontFormSubmitControl()) ->setValue('Update Tasks') diff --git a/src/applications/maniphest/controller/batch/__init__.php b/src/applications/maniphest/controller/batch/__init__.php index c6a14bffa0..6e7612179a 100644 --- a/src/applications/maniphest/controller/batch/__init__.php +++ b/src/applications/maniphest/controller/batch/__init__.php @@ -20,6 +20,7 @@ phutil_require_module('phabricator', 'infrastructure/javelin/markup'); phutil_require_module('phabricator', 'view/control/tokenizer'); phutil_require_module('phabricator', 'view/form/base'); phutil_require_module('phabricator', 'view/form/control/submit'); +phutil_require_module('phabricator', 'view/form/inset'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'markup'); diff --git a/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php b/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php index baf702ac6b..6f69faa18c 100644 --- a/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php +++ b/src/applications/owners/controller/edit/PhabricatorOwnersEditController.php @@ -233,10 +233,10 @@ final class PhabricatorOwnersEditController ? 'enabled' : 'disabled')) ->appendChild( - '

Paths

'. - '
'. - '
'. - javelin_render_tag( + id(new AphrontFormInsetView()) + ->setTitle('Paths') + ->addDivAttributes(array('id' => 'path-editor')) + ->setRightButton(javelin_render_tag( 'a', array( 'href' => '#', @@ -244,19 +244,16 @@ final class PhabricatorOwnersEditController 'sigil' => 'addpath', 'mustcapture' => true, ), - 'Add New Path'). - '
'. - '

Specify the files and directories which comprise this '. - 'package.

'. - '
'. - javelin_render_tag( - 'table', - array( - 'class' => 'owners-path-editor-table', - 'sigil' => 'paths', - ), - ''). - '
') + 'Add New Path')) + ->setDescription('Specify the files and directories which comprise '. + 'this package.') + ->setContent(javelin_render_tag( + 'table', + array( + 'class' => 'owners-path-editor-table', + 'sigil' => 'paths', + ), + ''))) ->appendChild( id(new AphrontFormTextAreaControl()) ->setLabel('Description') diff --git a/src/applications/owners/controller/edit/__init__.php b/src/applications/owners/controller/edit/__init__.php index bb97d51444..7411f46f57 100644 --- a/src/applications/owners/controller/edit/__init__.php +++ b/src/applications/owners/controller/edit/__init__.php @@ -23,6 +23,7 @@ phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/control/textarea'); phutil_require_module('phabricator', 'view/form/control/tokenizer'); phutil_require_module('phabricator', 'view/form/error'); +phutil_require_module('phabricator', 'view/form/inset'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php b/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php index 1f504557cf..149d762185 100644 --- a/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php +++ b/src/applications/project/controller/profileedit/PhabricatorProjectProfileEditController.php @@ -258,11 +258,9 @@ final class PhabricatorProjectProfileEditController ->setError($e_image) ->setCaption('Supported formats: '.implode(', ', $supported_formats))) ->appendChild( - '

Resources

'. - ''. - '
'. - '
'. - javelin_render_tag( + id(new AphrontFormInsetView()) + ->setTitle('Resources') + ->setRightButton(javelin_render_tag( 'a', array( 'href' => '#', @@ -270,18 +268,15 @@ final class PhabricatorProjectProfileEditController 'sigil' => 'add-resource', 'mustcapture' => true, ), - 'Add New Resource'). - '
'. - '

'. - '
'. - javelin_render_tag( + 'Add New Resource')) + ->addHiddenInput('resources', 'resources') + ->setContent(javelin_render_tag( 'table', array( 'sigil' => 'resources', 'class' => 'project-resource-table', ), - ''). - '
') + ''))) ->appendChild( id(new AphrontFormSubmitControl()) ->addCancelButton('/project/view/'.$project->getID().'/') diff --git a/src/applications/project/controller/profileedit/__init__.php b/src/applications/project/controller/profileedit/__init__.php index 9bb8a8d434..ee9bc895ff 100644 --- a/src/applications/project/controller/profileedit/__init__.php +++ b/src/applications/project/controller/profileedit/__init__.php @@ -31,6 +31,7 @@ phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/control/textarea'); phutil_require_module('phabricator', 'view/form/control/tokenizer'); phutil_require_module('phabricator', 'view/form/error'); +phutil_require_module('phabricator', 'view/form/inset'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phutil', 'utils'); diff --git a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php index b29d22ac18..1e5e0f790b 100644 --- a/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php +++ b/src/applications/repository/controller/edit/PhabricatorRepositoryEditController.php @@ -389,28 +389,25 @@ final class PhabricatorRepositoryEditController $form ->appendChild( - '

Basics

') - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel('Repository Name') - ->setValue($repository->getName())) - ->appendChild( - id(new AphrontFormSelectControl()) - ->setName('tracking') - ->setLabel('Tracking') - ->setOptions(array( - 'disabled' => 'Disabled', - 'enabled' => 'Enabled', - )) - ->setValue( - $repository->isTracked() + id(new AphrontFormInsetView()) + ->setTitle('Basics') + ->appendChild(id(new AphrontFormStaticControl()) + ->setLabel('Repository Name') + ->setValue($repository->getName())) + ->appendChild(id(new AphrontFormSelectControl()) + ->setName('tracking') + ->setLabel('Tracking') + ->setOptions(array( + 'disabled' => 'Disabled', + 'enabled' => 'Enabled', + )) + ->setValue( + $repository->isTracked() ? 'enabled' - : 'disabled')) - ->appendChild('
'); + : 'disabled'))); - $form->appendChild( - '

Remote URI

'. - '
'); + $inset = new AphrontFormInsetView(); + $inset->setTitle('Remote URI'); $clone_command = null; $fetch_command = null; @@ -435,7 +432,7 @@ final class PhabricatorRepositoryEditController 'Enter the URI to clone this repository from. It should look '. 'something like ssh://user@host.com/hg/example'; } - $form->appendChild( + $inset->appendChild( '

'.$instructions.'

'); } else if ($is_svn) { $instructions = @@ -444,12 +441,12 @@ final class PhabricatorRepositoryEditController 'the value in the Repository Root field. It should be a URI '. 'and look like http://svn.example.org/svn/ or '. 'svn+ssh://svn.example.com/svnroot/'; - $form->appendChild( + $inset->appendChild( '

'.$instructions.'

'); $uri_label = 'Repository Root'; } - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('uri') @@ -458,14 +455,14 @@ final class PhabricatorRepositoryEditController ->setValue($repository->getDetail('remote-uri')) ->setError($e_uri)); - $form->appendChild( + $inset->appendChild( '
'. 'If you want to connect to this repository over SSH, enter the '. 'username and private key to use. You can leave these fields blank if '. 'the repository does not use SSH.'. '
'); - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('ssh-login') @@ -490,7 +487,7 @@ final class PhabricatorRepositoryEditController 'look for a private key.')); if ($has_http_support) { - $form + $inset ->appendChild( '
'. 'If you want to connect to this repository over HTTP Basic Auth, '. @@ -509,7 +506,7 @@ final class PhabricatorRepositoryEditController ->setValue($repository->getDetail('http-pass'))); } - $form + $inset ->appendChild( '
'. 'To test your authentication configuration, save this '. @@ -523,33 +520,32 @@ final class PhabricatorRepositoryEditController 'from it.'. '
'); - $form->appendChild('
'); + $form->appendChild($inset); - $form->appendChild( - '

Importing Repository Information

'. - '
'); + $inset = new AphrontFormInsetView(); + $inset->setTitle('Repository Information'); if ($has_local) { - $form->appendChild( + $inset->appendChild( '

Select a path on local disk '. 'which the daemons should '.$clone_command.' the repository '. 'into. This must be readable and writable by the daemons, and '. 'readable by the webserver. The daemons will '.$fetch_command. ' and keep this repository up to date.

'); - $form->appendChild( + $inset->appendChild( id(new AphrontFormTextControl()) ->setName('path') ->setLabel('Local Path') ->setValue($repository->getDetail('local-path')) ->setError($e_path)); } else if ($is_svn) { - $form->appendChild( + $inset->appendChild( '

If you only want to parse one '. 'subpath of the repository, specify it here, relative to the '. 'repository root (e.g., trunk/ or projects/wheel/). '. 'If you want to parse multiple subdirectories, create a separate '. 'Phabricator repository for each one.

'); - $form->appendChild( + $inset->appendChild( id(new AphrontFormTextControl()) ->setName('svn-subpath') ->setLabel('Subpath') @@ -561,7 +557,7 @@ final class PhabricatorRepositoryEditController $branch_filter_str = implode( ', ', array_keys($repository->getDetail('branch-filter', array()))); - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('branch-filter') @@ -573,7 +569,7 @@ final class PhabricatorRepositoryEditController 'Example: master, release')); } - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('frequency') @@ -583,11 +579,10 @@ final class PhabricatorRepositoryEditController 'Number of seconds daemon should sleep between requests. Larger '. 'numbers reduce load but also decrease responsiveness.')); - $form->appendChild('
'); + $form->appendChild($inset); - $form->appendChild( - '

Application Configuration

'. - '
'); + $inset = new AphrontFormInsetView(); + $inset->setTitle('Application Configuration'); if ($has_branches) { @@ -598,7 +593,7 @@ final class PhabricatorRepositoryEditController $default_branch_name = 'master'; } - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('default-branch') @@ -612,7 +607,7 @@ final class PhabricatorRepositoryEditController 'Default branch to show in Diffusion.')); } - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('default-owners-path') @@ -623,7 +618,7 @@ final class PhabricatorRepositoryEditController '/')) ->setCaption('Default path in Owners tool.')); - $form + $inset ->appendChild( id(new AphrontFormSelectControl()) ->setName('herald-disabled') @@ -643,7 +638,7 @@ final class PhabricatorRepositoryEditController ->selectSymbolsWithoutLoading(); $parsers = ipull($parsers, 'name', 'name'); - $form + $inset ->appendChild( '

If you extend the commit '. 'message format, you can provide a new parser which will extract '. @@ -661,7 +656,7 @@ final class PhabricatorRepositoryEditController 'PhabricatorRepositoryDefaultCommitMessageDetailParser'))); if ($is_svn) { - $form + $inset ->appendChild( id(new AphrontFormTextControl()) ->setName('uuid') @@ -670,7 +665,7 @@ final class PhabricatorRepositoryEditController ->setCaption('Repository UUID from svn info.')); } - $form->appendChild('

'); + $form->appendChild($inset); $form ->appendChild( diff --git a/src/applications/repository/controller/edit/__init__.php b/src/applications/repository/controller/edit/__init__.php index a63242ea46..98961e1332 100644 --- a/src/applications/repository/controller/edit/__init__.php +++ b/src/applications/repository/controller/edit/__init__.php @@ -20,6 +20,7 @@ phutil_require_module('phabricator', 'view/form/control/submit'); phutil_require_module('phabricator', 'view/form/control/text'); phutil_require_module('phabricator', 'view/form/control/textarea'); phutil_require_module('phabricator', 'view/form/error'); +phutil_require_module('phabricator', 'view/form/inset'); phutil_require_module('phabricator', 'view/layout/panel'); phutil_require_module('phabricator', 'view/layout/sidenav'); diff --git a/src/view/form/inset/AphrontFormInsetView.php b/src/view/form/inset/AphrontFormInsetView.php new file mode 100644 index 0000000000..f9214892de --- /dev/null +++ b/src/view/form/inset/AphrontFormInsetView.php @@ -0,0 +1,123 @@ +title = $title; + return $this; + } + + public function setDescription($description) { + $this->description = $description; + return $this; + } + + public function setRightButton($button) { + $this->rightButton = $button; + return $this; + } + + public function setContent($content) { + $this->content = $content; + return $this; + } + + public function addHiddenInput($key, $value) { + if (is_array($value)) { + foreach ($value as $hidden_key => $hidden_value) { + $this->hidden[] = array($key.'['.$hidden_key.']', $hidden_value); + } + } else { + $this->hidden[] = array($key, $value); + } + return $this; + } + + public function addDivAttributes(array $attributes) { + $this->divAttributes = $attributes; + return $this; + } + + public function render() { + + $title = $hidden_inputs = $right_button = $desc = $content = ''; + + if ($this->title) { + $title = '

'.phutil_escape_html($this->title).'

'; + } + + $hidden_inputs = array(); + foreach ($this->hidden as $inp) { + list($key, $value) = $inp; + $hidden_inputs[] = phutil_render_tag( + 'input', + array( + 'type' => 'hidden', + 'name' => $key, + 'value' => $value, + )); + } + $hidden_inputs = implode("\n", $hidden_inputs); + + if ($this->rightButton) { + $right_button = phutil_render_tag( + 'div', + array( + 'style' => 'float: right;', + ), + $this->rightButton); + } + + if ($this->description) { + $desc = phutil_render_tag( + 'p', + array(), + $this->description); + + if ($right_button) { + $desc .= '
'; + } + } + + $div_attributes = $this->divAttributes; + $classes = array('aphront-form-inset'); + if (isset($div_attributes['class'])) { + $classes[] = $div_attributes['class']; + } + + $div_attributes['class'] = implode(' ', $classes); + + if ($this->content) { + $content = $this->content; + } + + return $title.phutil_render_tag( + 'div', + $div_attributes, + $hidden_inputs.$right_button.$desc.$content.$this->renderChildren()); + } +} diff --git a/src/view/form/inset/__init__.php b/src/view/form/inset/__init__.php new file mode 100644 index 0000000000..742d221d6d --- /dev/null +++ b/src/view/form/inset/__init__.php @@ -0,0 +1,14 @@ +