Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(invisibleMessage): introduce invisibleMessage util #3192

Merged
merged 6 commits into from
May 18, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions packages/base/src/types/InvisibleMessageMode.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import DataType from "./DataType.js";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should be called AnnouncementMode IMO


/**
* Enumeration for different mode behaviors of the <code>InvisibleMessage</code>.
* @private
*/
const InvisibleMessageModes = {

/**
* Indicates that updates to the region should be presented at the next graceful opportunity,
* such as at the end of reading the current sentence, or when the user pauses typing.
*/
Polite: "Polite",

/**
* Indicates that updates to the region have the highest priority and should be presented to the user immediately.
*/
Assertive: "Assertive",

};

class InvisibleMessageMode extends DataType {
static isValid(value) {
return !!InvisibleMessageModes[value];
}
}

InvisibleMessageMode.generateTypeAccessors(InvisibleMessageModes);

export default InvisibleMessageModes;
50 changes: 50 additions & 0 deletions packages/base/src/util/InvisibleMessage.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import InvisibleMessageMode from "../types/InvisibleMessageMode.js";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file does not belong to util in its current form. This is a module with side effects (creates DOM) and not a utility such as camelToKebabCase.

import getSingletonElementInstance from "./getSingletonElementInstance.js";

const styles = `position: absolute;
ivoplashkov marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole code is not CSP compliant. We cannot set CSS like this (innerText, cssText, etc...). Use the imperative APIs instead.

clip: rect(1px,1px,1px,1px);
user-select: none;
left: -1000px;
top: -1000px;
pointer-events: none;`;

const politeSpan = document.createElement("span");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code must be synchronized with the lifecycle of the framework. You can achieve this in one of two ways:

  • Make the creation of these spans lazy (during the call of the announce function - an only create the span that is announced - polite or assertive)
  • Export another function from this file, and call it in boot.js to create the spans safely after the HTML is loaded..

I prefer the first.

const assertiveSpan = document.createElement("span");

politeSpan.classList.add("ui5-invisiblemessage-polite");
assertiveSpan.classList.add("ui5-invisiblemessage-assertive");

politeSpan.setAttribute("aria-live", "polite");
assertiveSpan.setAttribute("aria-live", "assertive");

politeSpan.setAttribute("role", "alert");
assertiveSpan.setAttribute("role", "alert");

politeSpan.style.cssText = styles;
assertiveSpan.style.cssText = styles;

if (!politeSpan.parentElement) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't this be always true?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if you have pushed the changes.

getSingletonElementInstance("ui5-static-area").appendChild(politeSpan);
getSingletonElementInstance("ui5-static-area").appendChild(assertiveSpan);
}

/**
* Inserts the string into the respective span, depending on the mode provided.
*
* @param {string} message String to be announced by the screen reader.
* @param {sap.ui.core.InvisibleMessageMode} mode The mode to be inserted in the aria-live attribute.
*/
const announce = (message, mode) => {
// If no type is presented, fallback to polite announcement.
const span = mode === InvisibleMessageMode.Assertive ? assertiveSpan : politeSpan;

// Set textContent to empty string in order to trigger screen reader's announcement.
span.textContent = "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you'd need to make it empty and then assign the message?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone wants to announce the same message over and over again on some interaction, the screen reader won't be triggered if we don't empty the value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure that way, it'd be read out?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could would then become:
getSpan(mode).textContent = message;
where getSpan is a local function for this module that returns the span, if created, or creates and then returns it.

span.textContent = message;

if (mode !== InvisibleMessageMode.Assertive && mode !== InvisibleMessageMode.Polite) {
console.warn(`You have entered an invalid mode. Valid values are: "Polite" and "Assertive". The framework will automatically set the mode to "Polite".`); // eslint-disable-line
}
};

export default announce;
4 changes: 4 additions & 0 deletions packages/main/bundle.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ import applyDirection from "@ui5/webcomponents-base/dist/locale/applyDirection.j
import { attachDirectionChange } from "@ui5/webcomponents-base/dist/locale/directionChange.js";
import ResizeHandler from "@ui5/webcomponents-base/dist/delegate/ResizeHandler.js";
import * as defaultTexts from "./dist/generated/i18n/i18n-defaults.js";
import announce from "@ui5/webcomponents-base/dist/util/invisibleMessage.js";

const testAssets = {
configuration : {
Expand All @@ -118,6 +119,9 @@ const testAssets = {
getAssetsPath,
setAssetsPath
},
invisibleMessage : {
announce,
},
getLocaleData,
applyDirection,
attachDirectionChange,
Expand Down
46 changes: 46 additions & 0 deletions packages/main/test/pages/base/InvisibleMessage.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<!DOCTYPE html>
<html>
<head>
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no">

<title>InvisibleMessage</title>

<script src="../../../webcomponentsjs/webcomponents-loader.js"></script>
<script src="../../../resources/bundle.esm.js" type="module"></script>
<script nomodule src="../../../resources/bundle.es5.js"></script>
</head>

<body style="background-color: var(--sapBackgroundColor);">
<style>
ui5-textarea {
margin: 1rem 0;
}
</style>

<section class="group">
<ui5-title>InvisibleMessage announcement</ui5-title>
<ui5-textarea id="announce-textarea" placeholder="Enter text to be announced by the screen reader."></ui5-textarea>
<ui5-checkbox id="announce-checkbox" text="Assertive announcement"></ui5-checkbox>
<ui5-button id="announce-button" design="Emphasized" aria-expanded="true">Press me to announce.</ui5-button>
</section>

<script>
const button = document.querySelector("#announce-button");
const textarea = document.querySelector("#announce-textarea");
const checkbox = document.querySelector("#announce-checkbox");
let invisibleMessage;

button.addEventListener("click", function(event) {
invisibleMessage = window["sap-ui-webcomponents-bundle"].invisibleMessage;

if (checkbox.checked) {
invisibleMessage.announce(textarea.value, "Assertive")
} else {
invisibleMessage.announce(textarea.value)
}
});
</script>
</body>
</html>
34 changes: 34 additions & 0 deletions packages/main/test/specs/base/InvisibleMessage.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
const assert = require("chai").assert;
const PORT = require("../_port.js");

describe("InvisibleMessage", () => {
before(() => {
browser.url(`http://localhost:${PORT}/test-resources/pages/base/InvisibleMessage.html`);
});

it("Initial rendering", () => {
const politeSpan = browser.$(".ui5-invisiblemessage-polite");
const assertiveSpan = browser.$(".ui5-invisiblemessage-assertive");

assert.ok(politeSpan, "polite span is rendered");
assert.ok(assertiveSpan, "assertive span not rendered");
});

it("String annoucement", () => {
const politeSpan = browser.$(".ui5-invisiblemessage-polite");
const assertiveSpan = browser.$(".ui5-invisiblemessage-assertive");
const button = browser.$("#announce-button");
const checkBox = browser.$("#announce-checkbox");

browser.execute(() => {
document.getElementById("announce-textarea").value = "announcement";
});

button.click();
checkBox.click();
button.click();

assert.ok(politeSpan.getHTML().indexOf("announcement") > -1, "value has been announced");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not true "value has been announced"
It is true that it has been rendered

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check if you have pushed the changes.

assert.ok(assertiveSpan.getHTML().indexOf("announcement") > -1, "value has been announced");
});
});