Skip to content

Commit

Permalink
feat(ui5-panel): enable configuring the heading level (#1504)
Browse files Browse the repository at this point in the history
**Background**
Our native title used to be "h1", which is indeed wrong. In openui5 it is "h2" and it is not configurable. But, in the related issue the author wants to be able to change the title level in general. Although we provide the possibility of a custom header, the author is asking for this feature to make use of the fact that the the entire native header is clickable and focusable, which is not the case with the custom one.
**Options**
So, we either reject this and (1) hardcode it to "h2" as in openui5, (2) introduce a property as in this change, or (3) make another slot for the title, but then we will have the current "header" slot and "headerTitle" slot and it might be confusing.
**In this change:**
- [refactoring] Use div with role="heading" and "aria-level" attrs, instead of "h" tag to  enable the configuration of the level. Reuse the existing TitleLevel enum ("H1" to "H6") and extract the number part, but AriaLevel type ("1" - "6") is also an option.
- [FIX] sync the parameters in the latest visual design and change few variables
- [FIX] title now truncates, previously it used to wrap.

Fixes: #1495
  • Loading branch information
ilhan007 authored Apr 21, 2020
1 parent 746f907 commit 710053b
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 9 deletions.
8 changes: 6 additions & 2 deletions packages/main/src/Panel.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,12 @@
{{#if _hasHeader}}
<slot name="header"></slot>
{{else}}
<div class="ui5-panel-header-content">
<h1 class="ui5-panel-header-title">{{headerText}}</h1>
<div
role="heading"
aria-level="{{headerAriaLevel}}"
class="ui5-panel-header-title"
>
{{headerText}}
</div>
{{/if}}
</div>
Expand Down
19 changes: 19 additions & 0 deletions packages/main/src/Panel.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { getAnimationMode } from "@ui5/webcomponents-base/dist/config/AnimationM
import { fetchI18nBundle, getI18nBundle } from "@ui5/webcomponents-base/dist/i18nBundle.js";
import "@ui5/webcomponents-icons/dist/icons/navigation-right-arrow.js";
import Button from "./Button.js";
import TitleLevel from "./types/TitleLevel.js";
import PanelAccessibleRole from "./types/PanelAccessibleRole.js";
import PanelTemplate from "./generated/templates/PanelTemplate.lit.js";

Expand Down Expand Up @@ -102,6 +103,20 @@ const metadata = {
defaultValue: PanelAccessibleRole.Form,
},

/**
* Defines the "aria-level" of <code>ui5-panel</code> heading,
* set by the <code>headerText</code>.
* <br><br>
* Available options are: <code>"H6"</code> to <code>"H1"</code>.
* @type {TitleLevel}
* @defaultvalue "H2"
* @public
*/
headerLevel: {
type: TitleLevel,
defaultValue: TitleLevel.H2,
},

/**
* @private
*/
Expand Down Expand Up @@ -347,6 +362,10 @@ class Panel extends UI5Element {
};
}

get headerAriaLevel() {
return this.headerLevel.slice(1);
}

get headerTabIndex() {
return (this.header.length || this.fixed) ? "-1" : "0";
}
Expand Down
12 changes: 10 additions & 2 deletions packages/main/src/themes/Panel.css
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,17 @@
transform: rotate(90deg);
}

:host([fixed]) .ui5-panel-header-title {
width: 100%;
}

.ui5-panel-header-title {
font-family: var(--sapFontFamily);
font-size: var(--_ui5_panel_header_title_size);
width: calc(100% - var(--_ui5_panel_button_root_width));
overflow: hidden;
text-overflow: ellipsis;
white-space: nowrap;
font-family: var(--sapFontHeaderFamily);
font-size: var(--sapFontHeader5Size);
font-weight: normal;
color: var(--sapGroup_TitleTextColor);
}
Expand Down
1 change: 0 additions & 1 deletion packages/main/src/themes/base/Panel-parameters.css
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
:root {
--_ui5_panel_focus_border: 1px dotted var(--sapContent_FocusColor);
--_ui5_panel_header_height: 3rem;
--_ui5_panel_header_title_size: var(--sapMFontHeader4Size);
--_ui5_panel_button_root_width: 3rem;
}
1 change: 0 additions & 1 deletion packages/main/src/themes/sap_fiori_3/Panel-parameters.css
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@

:root {
--_ui5_panel_header_height: 2.75rem;
--_ui5_panel_header_title_size: var(--sapMFontHeader5Size);
--_ui5_panel_button_root_width: 2.75rem;
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@

:root {
--_ui5_panel_header_height: 2.75rem;
--_ui5_panel_header_title_size: var(--sapMFontHeader5Size);
--_ui5_panel_button_root_width: 2.75rem;
}
4 changes: 2 additions & 2 deletions packages/main/test/pages/Panel.html
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@

<br>

<ui5-panel id="panel-expandable" accessible-role="Complementary" header-text="Both expandable and expanded">
<ui5-panel id="panel-expandable" accessible-role="Complementary" header-text="Both expandable and expanded" header-level="H4">


<!-- Content -->
Expand Down Expand Up @@ -160,7 +160,7 @@

<br>

<ui5-panel id="panel1" collapsed header-text="Click to expand!">
<ui5-panel id="panel1" collapsed header-text="Click to expand!" header-level="H3">

<!-- Content -->
<ui5-title>This is a demo how to use the &quot;expand&quot; event.</ui5-title>
Expand Down
3 changes: 3 additions & 0 deletions packages/main/test/specs/Panel.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe("Panel general interaction", () => {

it("tests whether aria attributes are set correctly with native header", () => {
const header = browser.$("#panel1").shadow$(".ui5-panel-header");
const title = browser.$("#panel1").shadow$(".ui5-panel-header-title");
const button = browser.$("#panel1").shadow$(".ui5-panel-header-button");

assert.ok(!button.getAttribute("aria-expanded"), "aria-expanded shouldn't be set on the button");
Expand All @@ -109,6 +110,8 @@ describe("Panel general interaction", () => {

assert.ok(header.getAttribute("aria-expanded"), "aria-expanded should be set on the header");
assert.ok(header.getAttribute("aria-controls"), "aria-controls should be set on the header");

assert.strictEqual(title.getAttribute("aria-level"), "3", "title aria-level is set to 3 correctly");
});

it("tests whether aria attributes are set correctly in case of custom header", () => {
Expand Down

0 comments on commit 710053b

Please sign in to comment.