N°2847 - Refactor some early choices

- TWIG: Change calls to object methods from simple notation (eg. oObject.Id) to complete notation (eg. oObject.GetId()) to avoid confusion with use of arrays and variables (eg. aObject.sId) - UIBlock: $sId should not be first parameter as most of the time it can be ignored and generated by the system - NewsroomMenu: Rename method for something more less ambiguous - Html: Embed content in <div /> so we can easily find all such HTML fragments in the UI
2026-04-25 11:38:44 +02:00 · 2020-08-21 13:07:07 +02:00
parent 5320f69c44
commit d2bf4de84c
24 changed files with 94 additions and 65 deletions
--- a/pages/preferences.php
+++ b/pages/preferences.php
@@ -46,7 +46,7 @@ function DisplayPreferences($oP)
 	// User Language selection
 	//
 	//////////////////////////////////////////////////////////////////////////
-	$oUserLanguageBlock = new Panel('ibo-user-language-selection', Dict::S('UI:FavoriteLanguage'), array(), 'grey');
+	$oUserLanguageBlock = new Panel(Dict::S('UI:FavoriteLanguage'), array(), 'grey', 'ibo-user-language-selection');
 	$oUserLanguageStartForm = new Html('<form method="post">');
 	
  	$aLanguages = Dict::GetLanguages();
@@ -96,7 +96,7 @@ function DisplayPreferences($oP)
 	//
 	//////////////////////////////////////////////////////////////////////////

-	$oMiscSettingsBlock = new Panel('ibo-misc-settings', Dict::S('UI:FavoriteOtherSettings'), array(), 'grey');
+	$oMiscSettingsBlock = new Panel(Dict::S('UI:FavoriteOtherSettings'), array(), 'grey', 'ibo-misc-settings');

 	$oMiscSettingsStartForm = new Html('<form method="post" onsubmit="return ValidateOtherSettings()">');
 	
@@ -159,7 +159,7 @@ EOF
 	//
 	//////////////////////////////////////////////////////////////////////////

-	$oFavoriteOrganizationsBlock = new Panel('ibo-favorite-organizations', Dict::S('UI:FavoriteOrganizations'), array(), 'grey');
+	$oFavoriteOrganizationsBlock = new Panel(Dict::S('UI:FavoriteOrganizations'), array(), 'grey', 'ibo-favorite-organizations');

 	$sFavoriteOrganizationsHtml = '';
 	$sFavoriteOrganizationsHtml .= Dict::S('UI:FavoriteOrganizations+');
@@ -246,7 +246,7 @@ EOF
 	//
 	//////////////////////////////////////////////////////////////////////////

-	$oShortcutsBlock = new Panel('ibo-shortcuts', Dict::S('Menu:MyShortcuts'), array(), 'grey');
+	$oShortcutsBlock = new Panel(Dict::S('Menu:MyShortcuts'), array(), 'grey', 'ibo-shortcuts');
 	$sShortcutsHtml = '';
 	$oBMSearch = new DBObjectSearch('Shortcut');
 	$oBMSearch->AddCondition('user_id', UserRights::GetUserId(), '=');
@@ -349,7 +349,7 @@ EOF
 	$bNewsroomEnabled = (MetaModel::GetConfig()->Get('newsroom_enabled') !== false);
 	if ($bNewsroomEnabled && ($iCountProviders > 0))
 	{
-		$oNewsroomBlock = new Panel('ibo-newsroom', Dict::S('UI:Newsroom:Preferences'), array(), 'grey');
+		$oNewsroomBlock = new Panel(Dict::S('UI:Newsroom:Preferences'), array(), 'grey', 'ibo-newsroom');

 		$sNewsroomHtml = '';
 		$sNewsroomHtml .= '<form method="post">';
@@ -417,7 +417,7 @@ EOF
 	//
 	//////////////////////////////////////////////////////////////////////////

-	$oUserPicturePlaceHolderBlock = new Panel('ibo-user-picture-placeholder', Dict::S('UI:Preferences:ChooseAPlaceholder'), array(), 'grey');
+	$oUserPicturePlaceHolderBlock = new Panel(Dict::S('UI:Preferences:ChooseAPlaceholder'), array(), 'grey', 'ibo-user-picture-placeholder');

 	$sUserPicturesFolder = '../images/user-pictures/';
 	$sUserDefaultPicture = appUserPreferences::GetPref('user_picture_placeholder', 'default-placeholder.png');
--- a/sources/application/UI/Component/Alert/Alert.php
+++ b/sources/application/UI/Component/Alert/Alert.php
@@ -47,12 +47,12 @@ class Alert extends UIBlock
 	/**
 	 * Alert constructor.
 	 *
-	 * @param string $sId
 	 * @param string $sTitle
 	 * @param string $sMainText
 	 * @param string $sColor
+	 * @param string|null $sId
 	 */
-	public function __construct($sId, $sTitle = '', $sMainText = '', $sColor = 'secondary')
+	public function __construct($sTitle = '', $sMainText = '', $sColor = 'secondary', $sId = null)
 	{
 		$this->sTitle = $sTitle;
 		$this->sMainText = $sMainText;
--- a/sources/application/UI/Component/Breadcrumbs/Breadcrumbs.php
+++ b/sources/application/UI/Component/Breadcrumbs/Breadcrumbs.php
@@ -48,10 +48,10 @@ class Breadcrumbs extends UIBlock
 	/**
 	 * QuickCreate constructor.
 	 *
-	 * @param string $sId
 	 * @param array|null $aNewEntry
+	 * @param string|null $sId
 	 */
-	public function __construct($sId = null, $aNewEntry = null)
+	public function __construct($aNewEntry = null, $sId = null)
 	{
 		parent::__construct($sId);
 		$this->SetNewEntry($aNewEntry);
--- a/sources/application/UI/Component/GlobalSearch/GlobalSearch.php
+++ b/sources/application/UI/Component/GlobalSearch/GlobalSearch.php
@@ -51,12 +51,12 @@ class GlobalSearch extends UIBlock
 	/**
 	 * GlobalSearch constructor.
 	 *
-	 * @param string $sId
 	 * @param array $aLastQueries
+	 * @param string|null $sId
 	 *
 	 * @throws \Exception
 	 */
-	public function __construct($sId = null, $aLastQueries = [])
+	public function __construct($aLastQueries = [], $sId = null)
 	{
 		parent::__construct($sId);
 		$this->SetEndpoint(static::DEFAULT_ENDPOINT_REL_URL);
--- a/sources/application/UI/Component/GlobalSearch/GlobalSearchFactory.php
+++ b/sources/application/UI/Component/GlobalSearch/GlobalSearchFactory.php
@@ -43,6 +43,6 @@ class GlobalSearchFactory
 	{
 		$aLastClasses = GlobalSearchHelper::GetLastQueries();

-		return new GlobalSearch(GlobalSearch::BLOCK_CODE, $aLastClasses);
+		return new GlobalSearch($aLastClasses, GlobalSearch::BLOCK_CODE);
 	}
 }
--- a/sources/application/UI/Component/Html/Html.php
+++ b/sources/application/UI/Component/Html/Html.php
@@ -35,7 +35,6 @@ class Html extends UIBlock
 	// Overloaded constants
 	const BLOCK_CODE = 'ibo-html';
 	const HTML_TEMPLATE_REL_PATH = 'components/html/layout';
-	const JS_TEMPLATE_REL_PATH = 'components/html/layout';

 	/** @var string $sHtml */
 	protected $sHtml;
@@ -44,11 +43,12 @@ class Html extends UIBlock
 	 * Html constructor.
 	 *
 	 * @param string $sHtml
+	 * @param string|null $sId
 	 */
-	public function __construct($sHtml = '')
+	public function __construct($sHtml = '', $sId = null)
 	{
 		$this->sHtml = $sHtml;
-		parent::__construct();
+		parent::__construct($sId);
 	}

 	/**
--- a/sources/application/UI/Component/Panel/Panel.php
+++ b/sources/application/UI/Component/Panel/Panel.php
@@ -47,12 +47,12 @@ class Panel extends UIBlock
 	/**
 	 * Panel constructor.
 	 *
-	 * @param string $sId
 	 * @param string $sTitle
 	 * @param array $aSubBlocks
 	 * @param string $sColor
+	 * @param string|null $sId
 	 */
-	public function __construct($sId, $sTitle = '', $aSubBlocks = [], $sColor = 'secondary')
+	public function __construct($sTitle = '', $aSubBlocks = [], $sColor = 'secondary', $sId = null)
 	{
 		$this->sTitle = $sTitle;
 		$this->aSubBlocks = $aSubBlocks;
--- a/sources/application/UI/Component/PopoverMenu/NewsroomMenu/NewsroomMenu.php
+++ b/sources/application/UI/Component/PopoverMenu/NewsroomMenu/NewsroomMenu.php
@@ -32,9 +32,9 @@ use Combodo\iTop\Application\UI\Component\PopoverMenu\PopoverMenu;
 */
 class NewsroomMenu extends PopoverMenu
 {
+	// Overloaded constants
 	const HTML_TEMPLATE_REL_PATH = 'components/popover-menu/newsroom-menu/layout';
 	const JS_TEMPLATE_REL_PATH = 'components/popover-menu/newsroom-menu/layout';
-
 	const JS_FILES_REL_PATH = [
 		'js/components/newsroom-menu.js',
 	];
@@ -42,13 +42,25 @@ class NewsroomMenu extends PopoverMenu
 	/** @var array $aParams */
 	protected $aParams;

+	/**
+	 * Set all parameters at once
+	 *
+	 * @param array $aParams
+	 *
+	 * @return $this
+	 */
 	public function SetParams($aParams)
 	{
 		$this->aParams = $aParams;
 		return $this;
 	}

-	public function GetParams()
+	/**
+	 * Return all parameters as a JSON string
+	 *
+	 * @return false|string
+	 */
+	public function GetParamsAsJson()
 	{
 		return json_encode($this->aParams);
 	}
--- a/sources/application/UI/Component/PopoverMenu/NewsroomMenu/NewsroomMenuFactory.php
+++ b/sources/application/UI/Component/PopoverMenu/NewsroomMenu/NewsroomMenuFactory.php
@@ -46,20 +46,26 @@ class NewsroomMenuFactory
 	public static function MakeNewsroomMenuForNavigationMenu()
 	{
 			$oMenu = new NewsroomMenu('ibo-navigation-menu--notifications-menu');
-			$oMenu->SetParams(static::PrepareNewsForNewsroomMenu());
+			$oMenu->SetParams(static::PrepareParametersForNewsroomMenu());
 			
 			return $oMenu;
 	}

-
-	protected static function PrepareNewsForNewsroomMenu()
+	/**
+	 * Prepare parameters for the newsroom JS widget
+	 *
+	 * @return array
+	 * @throws \CoreException
+	 * @throws \CoreUnexpectedValue
+	 * @throws \MySQLException
+	 * @throws \OQLException
+	 */
+	protected static function PrepareParametersForNewsroomMenu()
 	{
-		$aItems = [];
-
 		$aProviderParams=[];
 		$oUser = UserRights::GetUserObject();
 		/**
-		 * @var iNewsroomProvider[] $aProviders
+		 * @var \iNewsroomProvider[] $aProviders
 		 */
 		$aProviders = MetaModel::EnumPlugins('iNewsroomProvider');
 		foreach($aProviders as $oProvider)
--- a/sources/application/UI/Component/PopoverMenu/PopoverMenu.php
+++ b/sources/application/UI/Component/PopoverMenu/PopoverMenu.php
@@ -37,7 +37,6 @@ class PopoverMenu extends UIBlock
 	// Overloaded constants
 	const BLOCK_CODE = 'ibo-popover-menu';
 	const HTML_TEMPLATE_REL_PATH = 'components/popover-menu/layout';
-	//const JS_TEMPLATE_REl_PATH = 'components/popover-menu/layout';
 	const JS_FILES_REL_PATH = [
 		'js/components/popover-menu.js',
 	];
--- a/sources/application/UI/Component/PopoverMenu/PopoverMenuItem/JsPopoverMenuItem.php
+++ b/sources/application/UI/Component/PopoverMenu/PopoverMenuItem/JsPopoverMenuItem.php
@@ -35,12 +35,19 @@ class JsPopoverMenuItem extends PopoverMenuItem
 	// Overloaded constants
 	const HTML_TEMPLATE_REL_PATH = 'components/popover-menu/item/mode_js';

+	/**
+	 * @see \JSPopupMenuItem::GetJsCode()
+	 * @return string
+	 */
 	public function GetJsCode()
 	{
 		return $this->oPopupMenuItem->GetJSCode();
 	}

-	/** @ignore */
+	/**
+	 * @see \JSPopupMenuItem::GetUrl()
+	 * @return string
+	 */
 	public function GetUrl()
 	{
 		return $this->oPopupMenuItem->GetUrl();
--- a/sources/application/UI/Component/PopoverMenu/PopoverMenuItem/UrlPopoverMenuItem.php
+++ b/sources/application/UI/Component/PopoverMenu/PopoverMenuItem/UrlPopoverMenuItem.php
@@ -33,13 +33,19 @@ class UrlPopoverMenuItem extends PopoverMenuItem
 	// Overloaded constants
 	const HTML_TEMPLATE_REL_PATH = 'components/popover-menu/item/mode_url';

-	/** @ignore */
+	/**
+	 * @see \URLPopupMenuItem::GetUrl()
+	 * @return string
+	 */
 	public function GetUrl()
 	{
 		return $this->oPopupMenuItem->GetUrl();
 	}
 	
-	/** @ignore */
+	/**
+	 * @see \URLPopupMenuItem::GetTarget()
+	 * @return string
+	 */
 	public function GetTarget()
 	{
 		return $this->oPopupMenuItem->GetTarget();
--- a/sources/application/UI/Component/QuickCreate/QuickCreate.php
+++ b/sources/application/UI/Component/QuickCreate/QuickCreate.php
@@ -56,13 +56,13 @@ class QuickCreate extends UIBlock
 	/**
 	 * QuickCreate constructor.
 	 *
-	 * @param string $sId
 	 * @param array $aLastClasses
+	 * @param string|null $sId
 	 *
 	 * @throws \CoreException
 	 * @throws \DictExceptionMissingString
 	 */
-	public function __construct($sId = null, $aLastClasses = [])
+	public function __construct($aLastClasses = [], $sId = null)
 	{
 		parent::__construct($sId);
 		$this->aAvailableClasses = UserRights::GetAllowedClasses(UR_ACTION_CREATE, array('bizmodel'), true);
--- a/sources/application/UI/Component/QuickCreate/QuickCreateFactory.php
+++ b/sources/application/UI/Component/QuickCreate/QuickCreateFactory.php
@@ -42,6 +42,6 @@ class QuickCreateFactory
 	{
 		$aLastClasses = QuickCreateHelper::GetLastClasses();

-		return new QuickCreate(QuickCreate::BLOCK_CODE,$aLastClasses);
+		return new QuickCreate($aLastClasses, QuickCreate::BLOCK_CODE);
 	}
 }
--- a/sources/application/UI/Layout/ActivityPanel/ActivityEntry/ActivityEntry.php
+++ b/sources/application/UI/Layout/ActivityPanel/ActivityEntry/ActivityEntry.php
@@ -74,14 +74,14 @@ class ActivityEntry extends UIBlock
 	 *
 	 * @param \DateTime $oDateTime
 	 * @param \User $sAuthorLogin
-	 * @param string $sContent
-	 * @param string $sIdCode
+	 * @param string|null $sContent
+	 * @param string|null $sId
 	 *
 	 * @throws \OQLException
 	 */
-	public function __construct(DateTime $oDateTime, $sAuthorLogin, $sContent = null, $sIdCode = null)
+	public function __construct(DateTime $oDateTime, $sAuthorLogin, $sContent = null, $sId = null)
 	{
-		parent::__construct($sIdCode);
+		parent::__construct($sId);

 		$this->SetType(static::DEFAULT_TYPE);
 		$this->SetDecorationClasses(static::DEFAULT_DECORATION_CLASSES);
--- a/sources/application/UI/Layout/ActivityPanel/ActivityEntry/CaseLogEntry.php
+++ b/sources/application/UI/Layout/ActivityPanel/ActivityEntry/CaseLogEntry.php
@@ -53,14 +53,14 @@ class CaseLogEntry extends ActivityEntry
 	 * @param \DateTime $oDateTime
 	 * @param \User $sAuthorLogin
 	 * @param string $sAttCode
-	 * @param string $sContentCode
+	 * @param string $sContent
 	 * @param string $sId
 	 *
 	 * @throws \OQLException
 	 */
-	public function __construct(DateTime $oDateTime, $sAuthorLogin, $sAttCode, $sContentCode, $sId = null)
+	public function __construct(DateTime $oDateTime, $sAuthorLogin, $sAttCode, $sContent, $sId = null)
 	{
-		parent::__construct($oDateTime, $sAuthorLogin, $sContentCode, $sId);
+		parent::__construct($oDateTime, $sAuthorLogin, $sContent, $sId);

 		$this->sAttCode = $sAttCode;
 		$this->SetCaseLogRank(static::DEFAULT_CASELOG_RANK);
--- a/sources/application/UI/Layout/NavigationMenu/NavigationMenu.php
+++ b/sources/application/UI/Layout/NavigationMenu/NavigationMenu.php
@@ -72,17 +72,18 @@ class NavigationMenu extends UIBlock
 	/**
 	 * NavigationMenu constructor.
 	 *
-	 * @param string|null $sId
 	 * @param \ApplicationContext $oAppContext
 	 * @param \Combodo\iTop\Application\UI\Component\PopoverMenu\PopoverMenu $oUserMenu
 	 * @param \Combodo\iTop\Application\UI\Component\PopoverMenu\NewsroomMenu\NewsroomMenu|null $oNewsroomMenu
+	 * @param string|null $sId
 	 *
 	 * @throws \CoreException
 	 * @throws \CoreUnexpectedValue
 	 * @throws \DictExceptionMissingString
 	 * @throws \MySQLException
+	 * @throws \Exception
 	 */
-	public function __construct($sId, ApplicationContext $oAppContext, PopoverMenu $oUserMenu, NewsroomMenu $oNewsroomMenu = null)
+	public function __construct(ApplicationContext $oAppContext, PopoverMenu $oUserMenu, NewsroomMenu $oNewsroomMenu = null, $sId = null)
 	{
 		parent::__construct($sId);

--- a/sources/application/UI/Layout/NavigationMenu/NavigationMenuFactory.php
+++ b/sources/application/UI/Layout/NavigationMenu/NavigationMenuFactory.php
@@ -54,10 +54,7 @@ class NavigationMenuFactory
 		}
 		
 		return new NavigationMenu(
-			NavigationMenu::BLOCK_CODE,
-			new ApplicationContext(),
-			PopoverMenuFactory::MakeUserMenuForNavigationMenu(),
-			$oNewsroomMenu
+			new ApplicationContext(), PopoverMenuFactory::MakeUserMenuForNavigationMenu(), $oNewsroomMenu, NavigationMenu::BLOCK_CODE
 		);
 	}
 }
--- a/sources/application/UI/Layout/TopBar/TopBar.php
+++ b/sources/application/UI/Layout/TopBar/TopBar.php
@@ -49,13 +49,14 @@ class TopBar extends UIBlock
 	/**
 	 * TopBar constructor.
 	 *
-	 * @param string $sId
-	 * @param \Combodo\iTop\Application\UI\Component\QuickCreate\QuickCreate $oQuickCreate
-	 * @param \Combodo\iTop\Application\UI\Component\GlobalSearch\GlobalSearch $oGlobalSearch
-	 * @param \Combodo\iTop\Application\UI\Component\Breadcrumbs\Breadcrumbs $oBreadcrumbs
+	 * @param string|null $sId
+	 * @param \Combodo\iTop\Application\UI\Component\QuickCreate\QuickCreate|null $oQuickCreate
+	 * @param \Combodo\iTop\Application\UI\Component\GlobalSearch\GlobalSearch|null $oGlobalSearch
+	 * @param \Combodo\iTop\Application\UI\Component\Breadcrumbs\Breadcrumbs|null $oBreadcrumbs
 	 */
-	public function __construct($sId = null, QuickCreate $oQuickCreate = null, GlobalSearch $oGlobalSearch = null, Breadcrumbs $oBreadcrumbs = null)
-	{
+	public function __construct(
+		$sId = null, QuickCreate $oQuickCreate = null, GlobalSearch $oGlobalSearch = null, Breadcrumbs $oBreadcrumbs = null
+	) {
 		parent::__construct($sId);

 		$this->oQuickCreate = $oQuickCreate;
--- a/sources/application/UI/Layout/TopBar/TopBarFactory.php
+++ b/sources/application/UI/Layout/TopBar/TopBarFactory.php
@@ -49,19 +49,19 @@ class TopBarFactory
 	{
 		$oTopBar = new TopBar(TopBar::BLOCK_CODE);

-		if(utils::GetConfig()->Get('quick_create.enabled') === true)
+		if (utils::GetConfig()->Get('quick_create.enabled') === true)
 		{
 			$oTopBar->SetQuickCreate(QuickCreateFactory::MakeFromUserHistory());
 		}

-		if(utils::GetConfig()->Get('global_search.enabled') === true)
+		if (utils::GetConfig()->Get('global_search.enabled') === true)
 		{
 			$oTopBar->SetGlobalSearch(GlobalSearchFactory::MakeFromUserHistory());
 		}

 		if(utils::GetConfig()->Get('breadcrumb.enabled') === true)
 		{
-			$oBreadcrumbs = new Breadcrumbs(Breadcrumbs::BLOCK_CODE, $aBreadcrumbsEntry);
+			$oBreadcrumbs = new Breadcrumbs($aBreadcrumbsEntry, Breadcrumbs::BLOCK_CODE);

 			$oTopBar->SetBreadcrumbs($oBreadcrumbs);
 		}
--- a/templates/components/button/layout.html.twig
+++ b/templates/components/button/layout.html.twig
@@ -1,6 +1,6 @@
-<button id="{{ oUIBlock.GetId() }}" class="ibo-button ibo-is-{{ oUIBlock.ActionType}} ibo-is-{{ oUIBlock.Color }}" type="{{ oUIBlock.Type }}" name="{{ oUIBlock.Name }}" value="{{ oUIBlock.Value }}">
-    {% if oUIBlock.IconClass is not empty %}
-      <span class="ibo-button-icon {{ oUIBlock.IconClass }}"></span>
+<button id="{{ oUIBlock.GetId() }}" class="ibo-button ibo-is-{{ oUIBlock.GetActionType()}} ibo-is-{{ oUIBlock.GetColor() }}" type="{{ oUIBlock.GetType() }}" name="{{ oUIBlock.GetName() }}" value="{{ oUIBlock.GetValue() }}">
+    {% if oUIBlock.GetIconClass() is not empty %}
+      <span class="ibo-button-icon {{ oUIBlock.GetIconClass() }}"></span>
    {% endif %}
-    {{ oUIBlock.Label }}
+    {{ oUIBlock.GetLabel() }}
 </button>
--- a/templates/components/html/layout.html.twig
+++ b/templates/components/html/layout.html.twig
@@ -1 +1 @@
-{{ oUIBlock.GetHtml()|raw }}
+<div class="ibo-html">{{ oUIBlock.GetHtml()|raw }}</div>
--- a/templates/components/popover-menu/newsroom-menu/layout.html.twig
+++ b/templates/components/popover-menu/newsroom-menu/layout.html.twig
@@ -1,4 +1,4 @@
 <div data-role="ibo-navigation-menu--notifications-menu--container">
-    <div {% if oUIBlock.Id is defined %}id="{{ oUIBlock.Id }}"{% endif %} class="ibo-popover-menu" data-role="ibo-popover-menu">
-    </div>
+	<div {% if oUIBlock.GetId() is defined %}id="{{ oUIBlock.GetId() }}"{% endif %} class="ibo-popover-menu"
+	     data-role="ibo-popover-menu"></div>
 </div>
--- a/templates/components/popover-menu/newsroom-menu/layout.js.twig
+++ b/templates/components/popover-menu/newsroom-menu/layout.js.twig
@@ -1,6 +1,6 @@
 // TODO: We need to find a clean way to launch this script only once the JS scripts are loaded
 document.addEventListener("DOMContentLoaded", function(){
 	setTimeout(function(){
-		$('#{{ oUIBlock.Id }}').newsroom_menu({{ oUIBlock.Params|raw }});
+		$('#{{ oUIBlock.GetId() }}').newsroom_menu({{ oUIBlock.GetParamsAsJson()|raw }});
 	}, 500);
 });