diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2268,7 +2268,7 @@ 'PhabricatorCustomFieldStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStorage.php', 'PhabricatorCustomFieldStorageQuery' => 'infrastructure/customfield/query/PhabricatorCustomFieldStorageQuery.php', 'PhabricatorCustomFieldStringIndexStorage' => 'infrastructure/customfield/storage/PhabricatorCustomFieldStringIndexStorage.php', - 'PhabricatorCustomHeaderConfigType' => 'applications/config/custom/PhabricatorCustomHeaderConfigType.php', + 'PhabricatorCustomLogoConfigType' => 'applications/config/custom/PhabricatorCustomLogoConfigType.php', 'PhabricatorDaemon' => 'infrastructure/daemon/PhabricatorDaemon.php', 'PhabricatorDaemonBulkJobController' => 'applications/daemon/controller/PhabricatorDaemonBulkJobController.php', 'PhabricatorDaemonBulkJobListController' => 'applications/daemon/controller/PhabricatorDaemonBulkJobListController.php', @@ -7014,7 +7014,7 @@ 'PhabricatorCustomFieldStorage' => 'PhabricatorLiskDAO', 'PhabricatorCustomFieldStorageQuery' => 'Phobject', 'PhabricatorCustomFieldStringIndexStorage' => 'PhabricatorCustomFieldIndexStorage', - 'PhabricatorCustomHeaderConfigType' => 'PhabricatorConfigOptionType', + 'PhabricatorCustomLogoConfigType' => 'PhabricatorConfigOptionType', 'PhabricatorDaemon' => 'PhutilDaemon', 'PhabricatorDaemonBulkJobController' => 'PhabricatorDaemonController', 'PhabricatorDaemonBulkJobListController' => 'PhabricatorDaemonBulkJobController', diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -328,6 +328,10 @@ 'metamta.re-prefix' => $global_settings_reason, 'metamta.vary-subjects' => $global_settings_reason, + + 'ui.custom-header' => pht( + 'This option has been replaced with `ui.logo`, which provides more '. + 'flexible configuration options.'), ); return $ancient_config; diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/PhabricatorConfigEditController.php --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/PhabricatorConfigEditController.php @@ -98,7 +98,8 @@ } } - $form = new AphrontFormView(); + $form = id(new AphrontFormView()) + ->setEncType('multipart/form-data'); $error_view = null; if ($errors) { @@ -144,9 +145,9 @@ } if ($option->getHidden() || $option->getLocked()) { - $control = null; + $controls = array(); } else { - $control = $this->renderControl( + $controls = $this->renderControls( $option, $display_value, $e_value); @@ -201,9 +202,9 @@ } } - $form - ->appendChild($control); - + foreach ($controls as $control) { + $form->appendControl($control); + } if (!$option->getLocked()) { $form->appendChild( @@ -279,23 +280,23 @@ $e_value = null; $errors = array(); - $value = $request->getStr('value'); - if (!strlen($value)) { - $value = null; - - $xaction->setNewValue( - array( - 'deleted' => true, - 'value' => null, - )); - - return array($e_value, $errors, $value, $xaction); - } - if ($option->isCustomType()) { $info = $option->getCustomObject()->readRequest($option, $request); list($e_value, $errors, $set_value, $value) = $info; } else { + $value = $request->getStr('value'); + if (!strlen($value)) { + $value = null; + + $xaction->setNewValue( + array( + 'deleted' => true, + 'value' => null, + )); + + return array($e_value, $errors, $value, $xaction); + } + $type = $option->getType(); $set_value = null; @@ -415,13 +416,13 @@ } } - private function renderControl( + private function renderControls( PhabricatorConfigOption $option, $display_value, $e_value) { if ($option->isCustomType()) { - $control = $option->getCustomObject()->renderControl( + $controls = $option->getCustomObject()->renderControls( $option, $display_value, $e_value); @@ -487,9 +488,11 @@ ->setError($e_value) ->setValue($display_value) ->setName('value'); + + $controls = array($control); } - return $control; + return $controls; } private function renderExamples(PhabricatorConfigOption $option) { diff --git a/src/applications/config/custom/PhabricatorConfigOptionType.php b/src/applications/config/custom/PhabricatorConfigOptionType.php --- a/src/applications/config/custom/PhabricatorConfigOptionType.php +++ b/src/applications/config/custom/PhabricatorConfigOptionType.php @@ -31,6 +31,16 @@ } + public function renderControls( + PhabricatorConfigOption $option, + $display_value, + $e_value) { + + $control = $this->renderControl($option, $display_value, $e_value); + + return array($control); + } + public function renderControl( PhabricatorConfigOption $option, $display_value, diff --git a/src/applications/config/custom/PhabricatorCustomHeaderConfigType.php b/src/applications/config/custom/PhabricatorCustomHeaderConfigType.php deleted file mode 100644 --- a/src/applications/config/custom/PhabricatorCustomHeaderConfigType.php +++ /dev/null @@ -1,48 +0,0 @@ -setViewer(PhabricatorUser::getOmnipotentUser()) - ->withPHIDs(array($value)) - ->executeOne(); - if (!$file) { - throw new Exception( - pht( - '%s is not a valid file PHID.', - $value)); - } - - $most_open_policy = PhabricatorPolicies::getMostOpenPolicy(); - if ($file->getViewPolicy() != $most_open_policy) { - throw new Exception( - pht( - 'Specified file %s has policy "%s" but should have policy "%s".', - $value, - $file->getViewPolicy(), - $most_open_policy)); - } - - if (!$file->isViewableImage()) { - throw new Exception( - pht( - 'Specified file %s is not a viewable image.', - $value)); - } - } - - public static function getExampleConfig() { - $config = 'PHID-FILE-abcd1234abcd1234abcd'; - return $config; - } - -} diff --git a/src/applications/config/custom/PhabricatorCustomLogoConfigType.php b/src/applications/config/custom/PhabricatorCustomLogoConfigType.php new file mode 100644 --- /dev/null +++ b/src/applications/config/custom/PhabricatorCustomLogoConfigType.php @@ -0,0 +1,118 @@ + 'optional string|null', + 'wordmarkText' => 'optional string|null', + )); + } + + public function readRequest( + PhabricatorConfigOption $option, + AphrontRequest $request) { + + $viewer = $request->getViewer(); + $view_policy = PhabricatorPolicies::POLICY_PUBLIC; + + if ($request->getBool('removeLogo')) { + $logo_image_phid = null; + } else if ($request->getFileExists('logoImage')) { + $logo_image = PhabricatorFile::newFromPHPUpload( + idx($_FILES, 'logoImage'), + array( + 'name' => 'logo', + 'authorPHID' => $viewer->getPHID(), + 'viewPolicy' => $view_policy, + 'canCDN' => true, + 'isExplicitUpload' => true, + )); + $logo_image_phid = $logo_image->getPHID(); + } else { + $logo_image_phid = self::getLogoImagePHID(); + } + + $wordmark_text = $request->getStr('wordmarkText'); + + $value = array( + 'logoImagePHID' => $logo_image_phid, + 'wordmarkText' => $wordmark_text, + ); + + $errors = array(); + $e_value = null; + + try { + $this->validateOption($option, $value); + } catch (Exception $ex) { + $e_value = pht('Invalid'); + $errors[] = $ex->getMessage(); + $value = array(); + } + + return array($e_value, $errors, $value, phutil_json_encode($value)); + } + + public function renderControls( + PhabricatorConfigOption $option, + $display_value, + $e_value) { + + try { + $value = phutil_json_decode($display_value); + } catch (Exception $ex) { + $value = array(); + } + + $logo_image_phid = idx($value, 'logoImagePHID'); + $wordmark_text = idx($value, 'wordmarkText'); + + $controls = array(); + + // TODO: This should be a PHUIFormFileControl, but that currently only + // works in "workflow" forms. It isn't trivial to convert this form into + // a workflow form, nor is it trivial to make the newer control work + // in non-workflow forms. + $controls[] = id(new AphrontFormFileControl()) + ->setName('logoImage') + ->setLabel(pht('Logo Image')); + + if ($logo_image_phid) { + $controls[] = id(new AphrontFormCheckboxControl()) + ->addCheckbox( + 'removeLogo', + 1, + pht('Remove Custom Logo')); + } + + $controls[] = id(new AphrontFormTextControl()) + ->setName('wordmarkText') + ->setLabel(pht('Wordmark')) + ->setPlaceholder(pht('Phabricator')) + ->setValue($wordmark_text); + + return $controls; + } + + +} diff --git a/src/applications/config/option/PhabricatorUIConfigOptions.php b/src/applications/config/option/PhabricatorUIConfigOptions.php --- a/src/applications/config/option/PhabricatorUIConfigOptions.php +++ b/src/applications/config/option/PhabricatorUIConfigOptions.php @@ -20,9 +20,6 @@ } public function getOptions() { - $custom_header_example = - PhabricatorCustomHeaderConfigType::getExampleConfig(); - $experimental_link = 'https://secure.phabricator.com/T4214'; $options = array( 'blindigo' => 'blindigo', 'red' => 'red', @@ -48,11 +45,24 @@ ] EOJSON; + $logo_type = 'custom:PhabricatorCustomLogoConfigType'; + return array( $this->newOption('ui.header-color', 'enum', 'blindigo') ->setDescription( pht('Sets the default color scheme of Phabricator.')) ->setEnumOptions($options), + $this->newOption('ui.logo', $logo_type, array()) + ->setSummary( + pht('Customize the logo and wordmark text in the header.')) + ->setDescription( + pht( + "Customize the logo image and text which appears in the main ". + "site header:\n\n". + " - **Logo Image**: Upload a new 80 x 80px image to replace the ". + "Phabricator logo in the site header.\n\n". + " - **Wordmark**: Choose new text to display next to the logo. ". + "By default, the header displays //Phabricator//.\n\n")), $this->newOption('ui.footer-items', 'list', array()) ->setSummary( pht( @@ -69,39 +79,6 @@ " omit this if you just want a piece of text, like a copyright ". " notice.")) ->addExample($example, pht('Basic Example')), - $this->newOption('ui.custom-wordmark', 'string', array()) - ->setSummary( - pht( - 'Customize the text next to the logo.')) - ->setDescription( - pht( - "Allows you to change the text (Phabricator by default) ". - "next to the Phabricator logo.\n\n")), - $this->newOption( - 'ui.custom-header', - 'custom:PhabricatorCustomHeaderConfigType', - null) - ->setSummary( - pht('Customize the Phabricator logo.')) - ->setDescription( - pht('You can customize the Phabricator logo by specifying the '. - 'phid for a viewable image you have uploaded to Phabricator '. - 'via the [[ /file/ | Files application]]. This image should '. - 'be:'."\n". - ' - 80px X 80px; while not enforced, images with these '. - 'dimensions will look best across devices.'."\n". - ' - have view policy public if [[ '. - '/config/edit/policy.allow-public | `policy.allow-public`]] '. - 'is true and otherwise view policy user; mismatches in these '. - 'policy settings will result in a broken logo for some users.'. - "\n\n". - 'You should restart Phabricator after updating this value '. - 'to see this change take effect.'. - "\n\n". - 'As this feature is experimental, please read [[ %s | T4214 ]] '. - 'for up to date information.', - $experimental_link)) - ->addExample($custom_header_example, pht('Valid Config')), ); } diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php --- a/src/view/page/menu/PhabricatorMainMenuView.php +++ b/src/view/page/menu/PhabricatorMainMenuView.php @@ -297,13 +297,13 @@ } private function renderPhabricatorLogo() { - $style_logo = null; - $logo = null; - $custom_header = PhabricatorEnv::getEnvConfig('ui.custom-header'); - $custom_wordmark = PhabricatorEnv::getEnvConfig('ui.custom-wordmark'); + $custom_header = PhabricatorCustomLogoConfigType::getLogoImagePHID(); + + $logo_style = array(); if ($custom_header) { $cache = PhabricatorCaches::getImmutableCache(); - $cache_key_logo = 'ui.custom-header.logo-phid.v1.'.$custom_header; + $cache_key_logo = 'ui.custom-header.logo-phid.v3.'.$custom_header; + $logo_uri = $cache->getKey($cache_key_logo); if (!$logo_uri) { $file = id(new PhabricatorFileQuery()) @@ -315,31 +315,32 @@ $cache->setKey($cache_key_logo, $logo_uri); } } - if ($logo_uri) { - $style_logo = - 'background-size: 40px 40px; '. - 'background-position: 0px 0px; '. - 'background-image: url('.$logo_uri.');'; - } - $logo = phutil_tag( - 'span', - array( - 'class' => 'phabricator-main-menu-logo', - 'style' => $style_logo, - ), - ''); + $logo_style[] = 'background-size: 40px 40px;'; + $logo_style[] = 'background-position: 0 0;'; + $logo_style[] = 'background-image: url('.$logo_uri.')'; } - if (!$logo) { - $logo = phutil_tag( - 'span', - array( - 'class' => 'phabricator-wordmark', - ), - (($custom_wordmark) ? $custom_wordmark : pht('Phabricator'))); + $logo_node = phutil_tag( + 'span', + array( + 'class' => 'phabricator-main-menu-eye', + 'style' => implode(' ', $logo_style), + )); + + + $wordmark_text = PhabricatorCustomLogoConfigType::getLogoWordmark(); + if (!strlen($wordmark_text)) { + $wordmark_text = pht('Phabricator'); } + $wordmark_node = phutil_tag( + 'span', + array( + 'class' => 'phabricator-wordmark', + ), + $wordmark_text); + return phutil_tag( 'a', array( @@ -353,13 +354,8 @@ 'aural' => true, ), pht('Home')), - phutil_tag( - 'span', - array( - 'class' => 'phabricator-main-menu-eye', - ), - ''), - $logo, + $logo_node, + $wordmark_node, )); }