Skip to content

Commit

Permalink
web: Stop using <abbr /> HTML tag (#1149)
Browse files Browse the repository at this point in the history
Some time ago, we started using the `<abbr>` tag for providing a way to
see the full text of an acronym or abbreviation.

However, at some point in the development of #1138, we realized that
even though it is a native HTML tag, it might not be as accessible as we
originally believed. In fact, the doubt arose when considering keyboard
users: how can they ask the browser to show the title attribute? By
default, they can't.

Having a look to [tests recently made by Adrian
Roselli](https://adrianroselli.com/2024/01/using-abbr-element-with-title-attribute.html),
it can be seen that, among other issues, it's commonly problematic for
keyboard and touch users. In Adrian's words:

> Exposure continues to be inconsistent across browsers and assistive
technologies. Some set of users will always miss some piece of
information.

In addition, such an element increases the risk of overusing it by
encouraging us to over explain acronyms, as @dgdavid almost did with
_Btrfs_, which he wanted to wrap into an `abbr` tag. As other examples,
_XFS_ and _USB_ are acronyms but it isn't actually helpful for readers
to being able to read _Extended File System_ and _Universal Serial Bus_
in the context in which they are used. Similar happens with _LVM_: if
users do not know the technology it represents, reading _Logical Volume
Manager_ hardly can help them.

Thus, it's better to stop using `<abbr />` and evaluate each particular
case when an acronym is added to the UI. If its long version could be
helpful for users we should follow the Adrian recommendation:

> Explain abbreviations, acronyms, initialisms, numeronyms, etc. on
first use and then feel free to fall back to the shortened form.

It could be the case of "Full Disk Encryption (FDE)".
  • Loading branch information
dgdavid authored Apr 15, 2024
2 parents f5946ff + 66e259c commit 1fcc535
Show file tree
Hide file tree
Showing 8 changed files with 35 additions and 42 deletions.
7 changes: 7 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
-------------------------------------------------------------------
Mon Apr 15 10:52:14 UTC 2024 - David Diaz <[email protected]>

- Stop using `<abbr>` tag because "its exposure continues to be
inconsistent across browsers and assistive technologies"
(gh#openSUSE/agama#1149).

-------------------------------------------------------------------
Mon Apr 15 07:14:35 UTC 2024 - David Diaz <[email protected]>

Expand Down
12 changes: 6 additions & 6 deletions web/src/components/core/InstallationFinished.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,12 @@ const TpmHint = () => {
onToggle={() => setIsExpanded(!isExpanded)}
toggleText={isExpanded ? _("Hide details") : _("See more details")}
>
<Text
dangerouslySetInnerHTML={{
// TRANSLATORS: Do not translate 'abbr' and 'title', they are part of the HTML markup
__html: _("The final step to configure the <abbr title='Trusted Platform Module'>TPM</abbr> to automatically open encrypted devices will take place during the first boot of the new system. For that to work, the machine needs to boot directly to the new boot loader.")
}}
/>
{
// TRANSLATORS: "Trusted Platform Module" is the name of the technology and "TPM" its abbreviation
_("The final step to configure the Trusted Platform Module (TPM) to automatically \
open encrypted devices will take place during the first boot of the new system. For that to work, \
the machine needs to boot directly to the new boot loader.")
}
</ExpandableSection>
</div>
</Alert>
Expand Down
5 changes: 2 additions & 3 deletions web/src/components/overview/StorageSection.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ const ProposalSummary = ({ proposal }) => {

if (result.settings.target === "NEW_LVM_VG") {
// TRANSLATORS: Part of the message describing where the system will be installed.
// Do not translate 'abbr' and 'title', they are part of the HTML markup.
const vg = _("<abbr title='Logical Volume Manager'>LVM</abbr> volume group");
const vg = _("Logical Volume Manager (LVM) volume group");
const pvDevices = result.settings.targetPVDevices;
const fullMsg = (policy, num_pvs) => {
switch (policy) {
Expand Down Expand Up @@ -110,7 +109,7 @@ const ProposalSummary = ({ proposal }) => {
return (
<Text>
<span dangerouslySetInnerHTML={{ __html: msg1 }} />
<Em>{ label(pvDevices[0]) }</Em>
<Em>{label(pvDevices[0])}</Em>
<span dangerouslySetInnerHTML={{ __html: msg2 }} />
</Text>
);
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/storage/EncryptionField.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import EncryptionSettingsDialog from "~/components/storage/EncryptionSettingsDia
// Field texts at root level to avoid redefinitions every time the component
// is rendered.
const LABEL = _("Encryption");
const DESCRIPTION = _("Full disk encryption allows to protect the information stored at \
const DESCRIPTION = _("Full Disk Encryption (FDE) allows to protect the information stored at \
the device, including data, programs, and system files.");
const VALUES = {
loading: <Skeleton width="150px" />,
Expand Down
14 changes: 7 additions & 7 deletions web/src/components/storage/EncryptionSettingsDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ import { EncryptionMethods } from "~/client/storage";
*/

const DIALOG_TITLE = _("Encryption");
const DIALOG_DESCRIPTION = _("Full disk encryption allows to protect the information stored at \
the device, including data, programs, and system files.");
const TPM_LABEL = _("Use the TPM to decrypt automatically on each boot");
const DIALOG_DESCRIPTION = _("Full Disk Encryption (FDE) allows to protect the information stored \
at the device, including data, programs, and system files.");
// TRANSLATORS: "Trusted Platform Module" is the name of the technology and TPM its abbreviation
const TPM_LABEL = _("Use the Trusted Platform Module (TPM) to decrypt automatically on each boot");
// TRANSLATORS: The word 'directly' is key here. For example, booting to the installer media and then choosing
// 'Boot from Hard Disk' from there will not work. Keep it sort (this is a hint in a form) but keep it clear.
// Do not translate 'abbr' and 'title', they are part of the HTML markup.
const TPM_EXPLANATION = _("The password will not be needed to boot and access the data if the \
<abbr title='Trusted Platform Module'>TPM</abbr> can verify the integrity of the system. \
TPM sealing requires the new system to be booted directly on its first run.");
TPM can verify the integrity of the system. TPM sealing requires the new system to be booted \
directly on its first run.");

/**
* Renders a dialog that allows the user change encryption settings
Expand Down Expand Up @@ -112,7 +112,7 @@ export default function EncryptionSettingsDialog({
<Checkbox
id="tpm_encryption_method"
label={TPM_LABEL}
description={<span dangerouslySetInnerHTML={{ __html: TPM_EXPLANATION }} />}
description={TPM_EXPLANATION}
isChecked={method === EncryptionMethods.TPM}
isDisabled={!isEnabled}
onChange={changeMethod}
Expand Down
10 changes: 5 additions & 5 deletions web/src/components/storage/EncryptionSettingsDialog.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ describe("EncryptionSettingsDialog", () => {
const switchField = screen.getByRole("switch", { name: "Encrypt the system" });
const passwordInput = screen.getByLabelText("Password");
const confirmationInput = screen.getByLabelText("Password confirmation");
const tpmCheckbox = screen.getByRole("checkbox", { name: /Use the TPM/ });
const tpmCheckbox = screen.getByRole("checkbox", { name: /Use.*TPM/ });
const acceptButton = screen.getByRole("button", { name: "Accept" });

expect(switchField).not.toBeChecked();
Expand Down Expand Up @@ -85,7 +85,7 @@ describe("EncryptionSettingsDialog", () => {
const passwordInput = screen.getByLabelText("Password");
const confirmationInput = screen.getByLabelText("Password confirmation");
const acceptButton = screen.getByRole("button", { name: "Accept" });
const tpmCheckbox = screen.getByRole("checkbox", { name: /Use the TPM/ });
const tpmCheckbox = screen.getByRole("checkbox", { name: /Use.*TPM/ });

await user.clear(passwordInput);
await user.type(passwordInput, "9876");
Expand Down Expand Up @@ -118,8 +118,8 @@ describe("EncryptionSettingsDialog", () => {

it("allows to stop using it", async () => {
const { user } = plainRender(<EncryptionSettingsDialog {...props} />);
const tpmCheckbox = screen.queryByRole("checkbox", { name: /Use the TPM/ });
const acceptButton = screen.queryByRole("button", { name: "Accept" });
const tpmCheckbox = screen.getByRole("checkbox", { name: /Use.*TPM/ });
const acceptButton = screen.getByRole("button", { name: "Accept" });
expect(tpmCheckbox).toBeChecked();
await user.click(tpmCheckbox);
expect(tpmCheckbox).not.toBeChecked();
Expand All @@ -137,7 +137,7 @@ describe("EncryptionSettingsDialog", () => {

it("does not render the TPM checkbox", () => {
plainRender(<EncryptionSettingsDialog {...props} />);
expect(screen.queryByRole("checkbox", { name: /Use the TPM/ })).toBeNull();
expect(screen.queryByRole("checkbox", { name: /Use.*TPM/ })).toBeNull();
});
});

Expand Down
25 changes: 6 additions & 19 deletions web/src/components/storage/InstallationDeviceField.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ import { sprintf } from "sprintf-js";
* @typedef {import ("~/client/storage").StorageDevice} StorageDevice
*/

const LABEL = _("Installation device");
// TRANSLATORS: The storage "Installation device" field's description.
const DESCRIPTION = _("Select the main disk or LVM Volume Group for installation.");

/**
* Generates the target value.
* @function
Expand All @@ -58,23 +62,6 @@ const targetValue = (target, targetDevice, targetPVDevices) => {
return _("No device selected yet");
};

/**
* Field description.
* @function
*
* @returns {React.ReactElement}
*/
const renderDescription = () => (
<span
dangerouslySetInnerHTML={{
// TRANSLATORS: The storage "Device" sections's description. Do not translate 'abbr' and
// 'title', they are part of the HTML markup.
__html: _("Select the main disk or <abbr title='Logical Volume Manager'>LVM</abbr> \
Volume Group for installation.")
}}
/>
);

const StorageTechSelector = () => {
return (
<ProposalPageMenu label={_("storage technologies")} />
Expand Down Expand Up @@ -128,9 +115,9 @@ export default function InstallationDeviceField({

return (
<SettingsField
label={_("Installation device")}
label={LABEL}
value={value}
description={renderDescription()}
description={DESCRIPTION}
onClick={openDialog}
>
{_("Prepare more devices by configuring advanced")} <StorageTechSelector />
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/storage/ProposalResultSection.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ const DevicesTreeTable = ({ devicesManager }) => {
};

const renderPTableType = (item) => {
// TODO: Create a map for partition table types and use an <abbr/> here.
// TODO: Create a map for partition table types.
const type = item.partitionTable?.type;
if (type) return <Tag><b>{type.toUpperCase()}</b></Tag>;
};
Expand Down

0 comments on commit 1fcc535

Please sign in to comment.