feat(ui5-side-navigation): add external link icon

Address review comments
SAP · Feb 6, 2024 · 620f229 · 620f229
1 parent c416eef
commit 620f229
Show file tree

Hide file tree

Showing 14 changed files with 127 additions and 111 deletions.
diff --git a/packages/fiori/src/SideNavigation.hbs b/packages/fiori/src/SideNavigation.hbs
@@ -143,20 +143,20 @@
 				{{#if icon}}
 					<ui5-icon class="ui5-sn-item-icon" name="{{icon}}"></ui5-icon>
 				{{/if}}
-				<div class="ui5-sn-item-text">{{text}}</div>
-				<ui5-icon class="ui5-sn-item-selection-icon"
-						  name="circle-task-2"
-				></ui5-icon>
+					<div class="ui5-sn-item-text">{{text}}</div>
+					<ui5-icon class="ui5-sn-item-selection-icon"
+							  name="circle-task-2"
+					></ui5-icon>
 				{{#if items.length}}
-				<ui5-icon class="ui5-sn-item-toggle-icon"
-						  name="{{_toggleIconName}}"
-						  @click="{{_onToggleClick}}"
-				></ui5-icon>
+					<ui5-icon class="ui5-sn-item-toggle-icon"
+							  name="{{_toggleIconName}}"
+							  @click="{{_onToggleClick}}"
+					></ui5-icon>
 				{{/if}}
 				{{#if _isExternalLink}}
-				<ui5-icon class="ui5-sn-item-external-link-icon"
-						  name="arrow-right"
-				></ui5-icon>
+					<ui5-icon class="ui5-sn-item-external-link-icon"
+							  name="arrow-right"
+					></ui5-icon>
 				{{/if}}
 			</a>
 		{{else}}
@@ -223,9 +223,9 @@
 										  name="circle-task-2"
 								></ui5-icon>
 								{{#if _isExternalLink}}
-								<ui5-icon class="ui5-sn-item-external-link-icon"
-							 			  name="arrow-right"
-								></ui5-icon>
+									<ui5-icon class="ui5-sn-item-external-link-icon"
+											  name="arrow-right"
+									></ui5-icon>
 								{{/if}}
 							</a>
 						{{else}}

diff --git a/packages/fiori/src/SideNavigationItemBase.ts b/packages/fiori/src/SideNavigationItemBase.ts
@@ -169,7 +169,7 @@ class SideNavigationItemBase extends UI5Element implements ITabbable {
 	}
 
 	get _isExternalLink() {
-		return this.href && this._target === "_blank";
+		return this.href && this.target === "_blank";
 	}
 
 	get sideNavigation() : SideNavigation | undefined {

diff --git a/packages/fiori/src/themes/SideNavigation.css b/packages/fiori/src/themes/SideNavigation.css
@@ -216,11 +216,12 @@
 }
 
 .ui5-sn-item-toggle-icon, .ui5-sn-item-external-link-icon {
-	color: var(--_ui5_side_navigation_expand_icon_color);
+	color: var(--_ui5_side_navigation_external_link_icon_color);
 	min-width: 2rem;
 	height: 0.875rem;
 }
 
+
 .ui5-sn-item-fixed .ui5-sn-item-toggle-icon, .ui5-sn-item-fixed .ui5-sn-item-external-link-icon {
 	display: none;
 }

diff --git a/packages/fiori/src/themes/base/SideNavigation-parameters.css b/packages/fiori/src/themes/base/SideNavigation-parameters.css
@@ -21,6 +21,7 @@
 	--_ui5_side_navigation_icon_color: var(--sapContent_IconColor);
 	--_ui5_side_navigation_icon_font_size: 1rem;
 	--_ui5_side_navigation_expand_icon_color: var(--sapContent_IconColor);
+	--_ui5_side_navigation_external_link_icon_color: var(--sapContent_LabelColor);
 	--_ui5_side_navigation_hover_border_style_color: none;
 	--_ui5_side_navigation_hover_border_width: 0;
 	--_ui5_side_navigation_group_border_style_color: solid var(--sapList_BorderColor);

diff --git a/packages/fiori/src/themes/sap_horizon/SideNavigation-parameters.css b/packages/fiori/src/themes/sap_horizon/SideNavigation-parameters.css
@@ -22,6 +22,7 @@
 	--_ui5_side_navigation_icon_color: var(--sapList_TextColor);
 	--_ui5_side_navigation_icon_font_size: 1.125rem;
 	--_ui5_side_navigation_expand_icon_color: var(--sapList_TextColor);
+	--_ui5_side_navigation_external_link_icon_color: var(--sapContent_LabelColor);
 	--_ui5_side_navigation_group_border_style_color: none;
 	--_ui5_side_navigation_item_height: 2.5rem;
 	--_ui5_side_navigation_item_border_radius: 0.375rem;

diff --git a/packages/fiori/src/themes/sap_horizon_dark/SideNavigation-parameters.css b/packages/fiori/src/themes/sap_horizon_dark/SideNavigation-parameters.css
@@ -22,6 +22,7 @@
 	--_ui5_side_navigation_icon_color: var(--sapList_TextColor);
 	--_ui5_side_navigation_icon_font_size: 1.125rem;
 	--_ui5_side_navigation_expand_icon_color: var(--sapList_TextColor);
+	--_ui5_side_navigation_external_link_icon_color: var(--sapContent_LabelColor);
 	--_ui5_side_navigation_group_border_style_color: none;
 	--_ui5_side_navigation_item_height: 2.5rem;
 	--_ui5_side_navigation_item_border_radius: 0.375rem;

diff --git a/packages/fiori/src/themes/sap_horizon_hcb/SideNavigation-parameters.css b/packages/fiori/src/themes/sap_horizon_hcb/SideNavigation-parameters.css
@@ -17,6 +17,7 @@
 
 	--_ui5_side_navigation_icon_font_size: 1.125rem;
 	--_ui5_side_navigation_expand_icon_color: var(--sapContent_NonInteractiveIconColor);
+	--_ui5_side_navigation_external_link_icon_color: var(--sapContent_NonInteractiveIconColor);
 	--_ui5_side_navigation_item_height: 2.5rem;
 	--_ui5_side_navigation_item_bottom_margin: 0.25rem;
 	--_ui5_side_navigation_item_bottom_margin_compact: 0.5rem;

diff --git a/packages/fiori/src/themes/sap_horizon_hcw/SideNavigation-parameters.css b/packages/fiori/src/themes/sap_horizon_hcw/SideNavigation-parameters.css
@@ -17,6 +17,7 @@
 
 	--_ui5_side_navigation_icon_font_size: 1.125rem;
 	--_ui5_side_navigation_expand_icon_color: var(--sapContent_NonInteractiveIconColor);
+	--_ui5_side_navigation_external_link_icon_color: var(--sapContent_LabelColor);
 	--_ui5_side_navigation_item_height: 2.5rem;
 	--_ui5_side_navigation_item_bottom_margin: 0.25rem;
 	--_ui5_side_navigation_item_bottom_margin_compact: 0.5rem;

diff --git a/packages/fiori/test/pages/SideNavigation.html b/packages/fiori/test/pages/SideNavigation.html
@@ -47,17 +47,22 @@
 				<ui5-side-navigation-sub-item icon="chain-link" text="Local" href="https://sap.com" target="_blank"></ui5-side-navigation-sub-item>
 				<ui5-side-navigation-sub-item text="Others"></ui5-side-navigation-sub-item>
 			</ui5-side-navigation-item>
-			<ui5-side-navigation-item text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-item>
+			<ui5-side-navigation-item id="externalLinkItem" text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-item>
 
-			<ui5-side-navigation-item id="item1" text="Home" icon="home" title="Home tooltip"></ui5-side-navigation-item>
-			<ui5-side-navigation-item id="item1" text="Home" icon="home" title="Home tooltip"></ui5-side-navigation-item>
+			<ui5-side-navigation-item id="item1" text="Home" icon="home" title="Home tooltip">
+				<ui5-side-navigation-sub-item icon="chain-link" text="Local" href="https://sap.com" target="_blank"></ui5-side-navigation-sub-item>
+				<ui5-side-navigation-sub-item text="Others"></ui5-side-navigation-sub-item>
+			</ui5-side-navigation-item>
+			<ui5-side-navigation-item disabled id="item1" text="Home" icon="home" title="Home tooltip"></ui5-side-navigation-item>
 
 			<!-- Fixed Items -->
 			<ui5-side-navigation-item id="fixedItem1" slot="fixedItems" text="Useful Links" icon="chain-link" title="Useful links tooltip">
 				<ui5-side-navigation-sub-item id="fixedItem11" text="Vacation Tool" title="Vacation Tool tooltip"></ui5-side-navigation-sub-item>
 				<ui5-side-navigation-sub-item id="fixedItem12" text="HR tool"></ui5-side-navigation-sub-item>
+				<ui5-side-navigation-sub-item text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-sub-item>
 			</ui5-side-navigation-item>
 			<ui5-side-navigation-item slot="fixedItems" text="History" icon="history"></ui5-side-navigation-item>
+			<ui5-side-navigation-item slot="fixedItems" text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-item>
 		</ui5-side-navigation>
 		<div class="inner-content">
 			<div>

diff --git a/packages/fiori/test/pages/SideNavigationOnly.html b/packages/fiori/test/pages/SideNavigationOnly.html
@@ -23,6 +23,7 @@
 			<ui5-side-navigation-sub-item href="#page1" target="_self" text="Local.............................................................."></ui5-side-navigation-sub-item>
 			<ui5-side-navigation-sub-item text="Others"></ui5-side-navigation-sub-item>
 		</ui5-side-navigation-item>
+		<ui5-side-navigation-item text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-item>
 		<ui5-side-navigation-item href="#locations" text="Locations" icon="locate-me"></ui5-side-navigation-item>
 		<ui5-side-navigation-item href="#locations" text="Locations" icon="locate-me"></ui5-side-navigation-item>
 		<ui5-side-navigation-item href="#locations" text="Locations" icon="locate-me"></ui5-side-navigation-item>
@@ -48,8 +49,9 @@
 		<ui5-side-navigation-item  disabled href="#locations" text="Locations" icon="tnt-v3/aggregator"></ui5-side-navigation-item>
 		<ui5-side-navigation-item href="#locations" text="Locations" icon="SAP-icons-v5/navigation-right-arrow"></ui5-side-navigation-item>
 		<ui5-side-navigation-item  text="Locations" icon="SAP-icons-v4/slim-arrow-up"></ui5-side-navigation-item>
-		<ui5-side-navigation-item  text="Locations" icon="SAP-icons-v4/favorite"></ui5-side-navigation-item>
-		<ui5-side-navigation-item  text="Locations" icon="SAP-icons-v4/less"></ui5-side-navigation-item>
+		<ui5-side-navigation-item disabled text="Locations" icon="SAP-icons-v4/favorite"></ui5-side-navigation-item>
+		<ui5-side-navigation-item disabled text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-item>
+		<ui5-side-navigation-item text="Locations" icon="SAP-icons-v4/less"></ui5-side-navigation-item>
 		<ui5-side-navigation-item href="#page1" target="_self" text="Locations" icon="group">
 			<ui5-side-navigation-sub-item icon="home" href="#page1" target="_self" text="Local.............................................................."></ui5-side-navigation-sub-item>
 			<ui5-side-navigation-sub-item icon="female" text="Others"></ui5-side-navigation-sub-item>
@@ -58,8 +60,10 @@
 		<ui5-side-navigation-item slot="fixedItems" text="Useful Links" icon="chain-link" title="Useful links tooltip">
 			<ui5-side-navigation-sub-item text="Vacation Tool" title="Vacation Tool tooltip"></ui5-side-navigation-sub-item>
 			<ui5-side-navigation-sub-item text="HR tool"></ui5-side-navigation-sub-item>
+			<ui5-side-navigation-sub-item text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-sub-item>
 		</ui5-side-navigation-item>
 		<ui5-side-navigation-item slot="fixedItems" text="History" icon="history"></ui5-side-navigation-item>
+		<ui5-side-navigation-item text="External Link" icon="chain-link" href="https://sap.com" target="_blank"></ui5-side-navigation-item>
 	</ui5-side-navigation>
 
 	<div style="float: right">

diff --git a/packages/fiori/test/specs/SideNavigation.spec.js b/packages/fiori/test/specs/SideNavigation.spec.js
@@ -265,5 +265,12 @@ describe("Component Behavior", () => {
 			assert.notOk(await overflowItem.isDisplayed(), "Overflow button should not be available");
 
 		});
+
+		it("Tests external link items", async () => {
+			const sideNavigation = await browser.$("#sn1");
+			const items = await sideNavigation.shadow$$(".ui5-sn-flexible .ui5-sn-item");
+
+			assert.ok(await items[4].$(".ui5-sn-item-external-link-icon").isExisting(), "External link icon is rendered");
+		});
 	});
 });
diff --git a/packages/main/src/NavigationMenu.hbs b/packages/main/src/NavigationMenu.hbs
@@ -47,7 +47,7 @@
 	>
 	{{#if _currentItems.length}}
 		<ui5-list
-            accessible-role="tree"
+			accessible-role="tree"
 			id="{{_id}}-menu-list"
 			mode="None"
 			?busy="{{busy}}"
@@ -57,93 +57,80 @@
 			@mouseover="{{_busyMouseOver}}"
 		>
 		{{#each _currentItems}}
-            {{#if this.item.href}}
-            <ui5-li
-                .associatedItem="{{this.item}}"
-                id="{{../_id}}-menu-item-{{@index}}"
-                .icon="{{this.item.icon}}"
-                accessible-name={{this.item.ariaLabelledByText}}
-                accessible-role="none"
-                .additionalText="{{this.item._additionalText}}"
-                .ariaHasPopup={{this.ariaHasPopup}}
-                ?disabled={{this.item.disabled}}
-                ?starts-section={{this.item.startsSection}}
-                ?selected={{this.item.subMenuOpened}}
-                ?is-phone={{../isPhone}}
-                @mouseover={{../_itemMouseOver}}
-                @mouseout={{../_itemMouseOut}}
-                @keydown={{../_itemKeyDown}}
-                class="ui5-menu-item ui5-navigation-menu-item">
-                        <a role="treeitem"
-                            href="{{this.item.href}}"
-                            target="{{this.item.target}}">
-                            {{#if this.item.hasDummyIcon}}
-                                <div
-                                    class="ui5-menu-item-dummy-icon"
-                                >
-                                </div>
-                            {{/if}}
-                            {{this.item.text}}
-                            {{#if this.item.hasSubmenu}}
-                                <ui5-icon
-                                    part="icon"
-                                    name="slim-arrow-right"
-                                    class="ui5-menu-item-icon-end"
-                                >
-                                </ui5-icon>
-                            {{else if this.item._siblingsWithChildren}}
-                                <div
-                                    class="ui5-menu-item-no-icon-end"
-                                >
-                                </div>
-                            {{/if}}
+			{{#if this.item.href}}
+				<ui5-li
+					.associatedItem="{{this.item}}"
+					id="{{../_id}}-menu-item-{{@index}}"
+					.icon="{{this.item.icon}}"
+					accessible-name={{this.item.ariaLabelledByText}}
+					accessible-role="none"
+					.additionalText="{{this.item._additionalText}}"
+					.ariaHasPopup={{this.ariaHasPopup}}
+					?disabled={{this.item.disabled}}
+					?starts-section={{this.item.startsSection}}
+					?selected={{this.item.subMenuOpened}}
+					?is-phone={{../isPhone}}
+					@mouseover={{../_itemMouseOver}}
+					@mouseout={{../_itemMouseOut}}
+					@keydown={{../_itemKeyDown}}
+					class="ui5-menu-item ui5-navigation-menu-item">
+						<a role="treeitem"
+						   href="{{this.item.href}}"
+						   target="{{this.item.target}}">
+							{{#if this.item.hasDummyIcon}}
+								<div class="ui5-menu-item-dummy-icon"></div>
+							{{/if}}
+							{{this.item.text}}
+						 	{{#if this.item.hasSubmenu}}
+								<ui5-icon
+									part="icon"
+									name="slim-arrow-right"
+									class="ui5-menu-item-icon-end"
+								></ui5-icon>
+							{{else if this.item._siblingsWithChildren}}
+								<div class="ui5-menu-item-no-icon-end"></div>
+							{{/if}}
 							{{#if this.item.target}}
-							<ui5-icon class="ui5-sn-item-external-link-icon"
-							  name="arrow-right"
-							></ui5-icon>
+								<ui5-icon class="ui5-sn-item-external-link-icon"
+										  name="arrow-right"
+								></ui5-icon>
 							{{/if}}
-                        </a>
-                    </ui5-li>
-            {{else}}
-                    <ui5-li
-                    .associatedItem="{{this.item}}"
-                    class="ui5-menu-item ui5-navigation-menu-item"
-                    id="{{../_id}}-menu-item-{{@index}}"
-                    .icon="{{this.item.icon}}"
-                    accessible-name={{this.item.ariaLabelledByText}}
-                    accessible-role="treeitem"
-                    .additionalText="{{this.item._additionalText}}"
-                    .ariaHasPopup={{this.ariaHasPopup}}
-                    ?disabled={{this.item.disabled}}
-                    ?starts-section={{this.item.startsSection}}
-                    ?selected={{this.item.subMenuOpened}}
-                    ?is-phone={{../isPhone}}
-                    @mouseover={{../_itemMouseOver}}
-                    @mouseout={{../_itemMouseOut}}
-                    @keydown={{../_itemKeyDown}}
-                >
-                    {{#if this.item.hasDummyIcon}}
-                        <div
-                            class="ui5-menu-item-dummy-icon"
-                        >
-                        </div>
-                    {{/if}}
-                    {{this.item.text}}
-                    {{#if this.item.hasSubmenu}}
-                        <ui5-icon
-                            part="icon"
-                            name="slim-arrow-right"
-                            class="ui5-menu-item-icon-end"
-                        >
-                        </ui5-icon>
-                    {{else if this.item._siblingsWithChildren}}
-                        <div
-                            class="ui5-menu-item-no-icon-end"
-                        >
-                        </div>
-                    {{/if}}
-			        </ui5-li>
-                {{/if}}
+						</a>
+					</ui5-li>
+				{{else}}
+				<ui5-li
+					.associatedItem="{{this.item}}"
+					class="ui5-menu-item ui5-navigation-menu-item"
+					id="{{../_id}}-menu-item-{{@index}}"
+					.icon="{{this.item.icon}}"
+					accessible-name={{this.item.ariaLabelledByText}}
+					accessible-role="treeitem"
+					.additionalText="{{this.item._additionalText}}"
+					.ariaHasPopup={{this.ariaHasPopup}}
+					?disabled={{this.item.disabled}}
+					?starts-section={{this.item.startsSection}}
+					?selected={{this.item.subMenuOpened}}
+					?is-phone={{../isPhone}}
+					@mouseover={{../_itemMouseOver}}
+					@mouseout={{../_itemMouseOut}}
+					@keydown={{../_itemKeyDown}}
+				>
+				{{#if this.item.hasDummyIcon}}
+					<div class="ui5-menu-item-dummy-icon"></div>
+				{{/if}}
+				{{this.item.text}}
+				{{#if this.item.hasSubmenu}}
+					<ui5-icon
+						part="icon"
+						name="slim-arrow-right"
+						class="ui5-menu-item-icon-end"
+					>
+					</ui5-icon>
+				{{else if this.item._siblingsWithChildren}}
+					<div class="ui5-menu-item-no-icon-end"></div>
+				{{/if}}
+				</ui5-li>
+			{{/if}}
 		{{/each}}
 		</ui5-list>
 	{{else if busy}}

diff --git a/packages/main/src/themes/NavigationMenu.css b/packages/main/src/themes/NavigationMenu.css
@@ -69,7 +69,7 @@
 }
 
 .ui5-sn-item-external-link-icon {
-	color: var(--_ui5_side_navigation_expand_icon_color);
+	color: var(--_ui5_side_navigation_external_link_icon_color);
 	min-width: 2rem;
 	height: 0.875rem;
 }