Add tooltipText param to TopNav and LeftNav #2242
Conversation
rafalgamon
requested review from
alexandra-simeonova,
hardl,
JohannesDoberer,
ndricimrr,
stanleychh,
UlianaMunich and
wdoberschuetz
as code owners
There was a problem hiding this comment.
Worked fine 👍 .
-
However, noticed that it doesn't disable tooltips if tooltipText set to empty string.
-
Other thing is
defaults.tooltipTextis also missing. If I got it right from Philipps comment , one should be able to define.
navigation: {
defaults: {
tooltipText: false
},
...
}
Not sure on details of the defaults mechanism (boolean vs string), but can discuss this in daily.
test/e2e-test-application/e2e/tests/1-angular/navigation.spec.js
Outdated
Show resolved
Hide resolved
Co-authored-by: Aleksandra Simeonova <aleksandra.simeonova@sap.com>
Co-authored-by: Aleksandra Simeonova <aleksandra.simeonova@sap.com>
|
Global defaults.tooltipText and tooltipText on node level, both worked well. However, I couldn't see how to make it work on the Top navigation. Some small addition to the defaults.tooltipText documentation could be done, to clear the confusion on how it works together with the node level one. |
Co-authored-by: Ndricim Rrapi <ndricim.rrapi@sap.com>
core/src/navigation/LeftNav.html
Outdated
| @@ -267,7 +270,7 @@ | |||
| class="lui-side-nav__footer--icon {isSemiCollapsed ? 'sap-icon--open-command-field' : 'sap-icon--close-command-field'}" | |||
| on:click="{() => semiCollapsibleButtonClicked(this)}" | |||
| data-testid="semiCollapsibleButton" | |||
| title="{burgerTooltip}" | |||
| title="burgerTooltip" | |||
There was a problem hiding this comment.
i assume removing the curly braces here was not intended, please revert
| @@ -416,6 +416,22 @@ class NavigationHelpersClass { | |||
| reject(error); | |||
| }); | |||
| } | |||
|
|
|||
| generateTooltipText(node, translation) { | |||
There was a problem hiding this comment.
logic should be like follows:
let tt_text = node.tooltipText;
if(tt_text === undefined) {
tt_text = LuigiConfig.getConfigValue('navigation.defaults.tooltipText');
}
if(tt_text === undefined) {
return translation;
} else if(tt_text === false) {
return '';
} else {
return LuigiI18N.getTranslation(tt_text);
}
…falgamon/luigi into 2160-tooltiptext-for-navigation
|
|
||
| ### defaults.isolateView | ||
| - **type**: boolean | ||
| - **description**: renders all views in new frames. This setting overrides the same-domain frame reuse. | ||
| - **default**: the parameter **defaults.isolateView** is `false` by default, and you can overwrite it using the **isolateView** value on a single node level. | ||
|
|
||
| ### tooltipText |
There was a problem hiding this comment.
"tooltipText" docu should be under "Node Parameters", defaults.tooltipText can stay here
|
|
||
| ### defaults.isolateView | ||
| - **type**: boolean | ||
| - **description**: renders all views in new frames. This setting overrides the same-domain frame reuse. | ||
| - **default**: the parameter **defaults.isolateView** is `false` by default, and you can overwrite it using the **isolateView** value on a single node level. | ||
|
|
||
| ### tooltipText | ||
| - **type**: string | ||
| - **description**: allows to overwrite a **defaults.tooltipText** to any of the links and buttons of the left side navigation and top navigation. |
There was a problem hiding this comment.
don't refer to defaults here, defaults are just a convenience functionality.
Write something like
"Allows to set a custom tooltip text for this node or to disable the tooltip by setting the value to false"
…falgamon/luigi into 2160-tooltiptext-for-navigation
Description
Changes proposed in this pull request: