Skip to content

Commit

Permalink
[web] Prefer HTML <strong> element over CSS bold class. (#960)
Browse files Browse the repository at this point in the history
There are some sentences across the UI visually emphasized by using the
CSS `bold` class. Even though it's not entirely incorrect, after
revisiting the [HTML spec](https://html.spec.whatwg.org) it looks more
appropriate to use the
[`<strong>`](https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-strong-element)
element instead. By doing so, delivered content becomes more semantic
and preserves visual clues by default.

Please, note that apart from switching from `<span class="bold">` to
`<strong>` this set of changes also fixes which sentences are marked as
important. In some case it just chooses the right sentences, while in
others it just removes the emphasis because it is not needed.

Additionally, affected sentences were modified for adding the final dot
if missing.
  • Loading branch information
dgdavid authored Dec 26, 2023
2 parents bd09092 + 240d466 commit 5e882f8
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 38 deletions.
6 changes: 6 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
-------------------------------------------------------------------
Tue Dec 26 11:12:45 UTC 2023 - David Diaz <[email protected]>

- UI: Use the HTML <strong> element instead of a CSS class for
emphasizing content (gh#openSUSE/agama#960).

-------------------------------------------------------------------
Fri Dec 22 09:29:59 UTC 2023 - David Diaz <[email protected]>

Expand Down
22 changes: 7 additions & 15 deletions web/src/components/core/InstallButton.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,34 +23,26 @@ import React, { useState } from "react";
import { useInstallerClient } from "~/context/installer";

import { Button } from "@patternfly/react-core";
import { useNavigate } from "react-router-dom";
import { Link } from "react-router-dom";

import { If, Popup } from "~/components/core";
import { _ } from "~/i18n";

const InstallConfirmationPopup = ({ hasIssues, onAccept, onClose }) => {
const navigate = useNavigate();

const IssuesWarning = () => {
const IssuesLink = ({ text }) => {
return (
<Button variant="link" isInline onClick={() => navigate("/issues")}>
{text}
</Button>
);
};

// TRANSLATORS: the installer reports some errors,
// the text in square brackets [] is a clickable link
const [msgStart, msgLink, msgEnd] = _("There are some reported issues. \
Please, check [the list of issues] \
before proceeding with the installation.").split(/[[\]]/);

return (
<p className="bold">
{msgStart}
<IssuesLink text={msgLink} />
{msgEnd}
<p>
<strong>
{msgStart}
<Link to="/issues">{msgLink}</Link>
{msgEnd}
</strong>
</p>
);
};
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/core/InstallButton.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ describe("when the button is clicked and there are not errors", () => {
const button = await screen.findByRole("button", { name: "Install" });
await user.click(button);

await screen.findByRole("button", { name: /list of issues$/ });
await screen.findByRole("link", { name: /list of issues$/ });
});
});

Expand All @@ -100,7 +100,7 @@ describe("when the button is clicked and there are not errors", () => {
const button = await screen.findByRole("button", { name: "Install" });
await user.click(button);
await waitFor(() => {
const link = screen.queryByRole("button", { name: /list of issues$/ });
const link = screen.queryByRole("link", { name: /list of issues$/ });
expect(link).toBeNull();
});
});
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/network/NetworkPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { _ } from "~/i18n";
const NoWiredConnections = () => {
return (
<div className="stack">
<div className="bold">{_("No wired connections found")}</div>
<div>{_("No wired connections found.")}</div>
</div>
);
};
Expand All @@ -55,7 +55,7 @@ const NoWifiConnections = ({ wifiScanSupported, openWifiSelector }) => {

return (
<div className="stack">
<div className="bold">{_("No WiFi connections found")}</div>
<div>{_("No WiFi connections found.")}</div>
<div>{message}</div>
<If
condition={wifiScanSupported}
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/network/NetworkPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe("NetworkPage", () => {
installerRender(<NetworkPage />);

const section = await screen.findByRole("region", { name: "Wired networks" });
await within(section).findByText("No wired connections found");
await within(section).findByText("No wired connections found.");
});
});

Expand All @@ -102,7 +102,7 @@ describe("NetworkPage", () => {
installerRender(<NetworkPage />);

const section = await screen.findByRole("region", { name: "WiFi networks" });
await within(section).findByText("No WiFi connections found");
await within(section).findByText("No WiFi connections found.");
});

describe("and WiFi scan is supported", () => {
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/storage/ProposalSettingsSection.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ const InstallationDeviceField = ({
>
<If
condition={devices.length === 0}
then={<div className="bold">{_("No devices found")}</div>}
then={_("No devices found.")}
else={
<InstallationDeviceForm
id="bootDeviceForm"
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/storage/VolumeForm.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ const FsSelectOption = ({ fsOption }) => {

return (
<SelectOption value={fsOption} description={description()}>
<span className="bold">{label()}</span> <If condition={details()} then={details()} />
<strong>{label()}</strong> <If condition={details()} then={details()} />
</SelectOption>
);
};
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/storage/ZFCPPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ const ControllersSection = ({ client, manager, load = noop, isLoading = false })
const EmptyState = () => {
return (
<div className="stack">
<div className="bold">{_("No zFCP controllers found")}</div>
<div>{_("No zFCP controllers found.")}</div>
<div>{_("Please, try to read the zFCP devices again.")}</div>
{/* TRANSLATORS: button label */}
<Button variant="primary" onClick={load}>{_("Read zFCP devices")}</Button>
Expand Down Expand Up @@ -550,7 +550,7 @@ const DisksSection = ({ client, manager, isLoading = false }) => {

return (
<div className="stack">
<div className="bold">{_("No zFCP disks found")}</div>
<div>{_("No zFCP disks found.")}</div>
<If
condition={manager.getActiveControllers().length === 0}
then={<NoActiveControllers />}
Expand Down
6 changes: 3 additions & 3 deletions web/src/components/storage/ZFCPPage.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ describe("if there are not controllers", () => {
it("renders a button for reading zFCP devices", async () => {
installerRender(<ZFCPPage />);

await screen.findByText(/No zFCP controllers found/);
await screen.findByText("No zFCP controllers found.");
await screen.findByRole("button", { name: /Read zFCP devices/ });
});

Expand Down Expand Up @@ -264,7 +264,7 @@ describe("if there are not disks", () => {
it("renders a button for activating a disk", async () => {
installerRender(<ZFCPPage />);

await screen.findByText("No zFCP disks found");
await screen.findByText("No zFCP disks found.");
await screen.findByText(/try to activate a zFCP disk/);
screen.findByRole("button", { name: "Activate zFCP disk" });
});
Expand All @@ -277,7 +277,7 @@ describe("if there are not disks", () => {
it("does not render a button for activating a disk", async () => {
installerRender(<ZFCPPage />);

await screen.findByText("No zFCP disks found");
await screen.findByText("No zFCP disks found.");
await screen.findByText(/try to activate a zFCP controller/);
await waitFor(() => expect(screen.queryByRole("button", { name: "Activate zFCP disk" })).toBeNull());
});
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/storage/iscsi/TargetsSection.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export default function TargetsSection() {
if (state.nodes.length === 0) {
return (
<div className="stack">
<div className="bold">{_("No iSCSI targets found")}</div>
<div>{_("No iSCSI targets found.")}</div>
<div>{_("Please, perform an iSCSI discovery in order to find available iSCSI targets.")}</div>
{/* TRANSLATORS: button label, starts iSCSI discovery */}
<Button variant="primary" onClick={openDiscoverForm}>{_("Discover iSCSI targets")}</Button>
Expand Down
8 changes: 6 additions & 2 deletions web/src/components/users/FirstUser.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,12 @@ import { RowActions, PasswordAndConfirmationInput, Popup } from '~/components/co
const UserNotDefined = ({ actionCb }) => {
return (
<div className="stack">
<div className="bold">{_("No user defined yet")}</div>
<div>{_("Please, be aware that a user must be defined before installing the system to be able to log into it.")}</div>
<div>{_("No user defined yet.")}</div>
<div>
<strong>
{_("Please, be aware that a user must be defined before installing the system to be able to log into it.")}
</strong>
</div>
{/* TRANSLATORS: push button label */}
<Button variant="primary" onClick={actionCb}>{_("Define a user now")}</Button>
</div>
Expand Down
8 changes: 4 additions & 4 deletions web/src/components/users/FirstUser.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ beforeEach(() => {

it("allows defining a new user", async () => {
const { user } = installerRender(<FirstUser />);
await screen.findByText("No user defined yet");
await screen.findByText("No user defined yet.");
const button = await screen.findByRole("button", { name: "Define a user now" });
await user.click(button);

Expand Down Expand Up @@ -156,7 +156,7 @@ describe("when there is some issue with the user config provided", () => {
await waitFor(() => {
expect(screen.queryByText(/Something went wrong/i)).toBeInTheDocument();
expect(screen.queryByText(/There is an error/i)).toBeInTheDocument();
expect(screen.queryByText(/No user defined yet/i)).toBeInTheDocument();
expect(screen.queryByText("No user defined yet.")).toBeInTheDocument();
});
});
});
Expand Down Expand Up @@ -263,14 +263,14 @@ describe("when the user has been modified", () => {
const [mockFunction, callbacks] = createCallbackMock();
onUsersChangeFn = mockFunction;
installerRender(<FirstUser />);
await screen.findByText("No user defined yet");
await screen.findByText("No user defined yet.");

const [cb] = callbacks;
act(() => {
cb({ firstUser: { userName: "ytm", fullName: "YaST Team Member", autologin: false } });
});

const noUserInfo = await screen.queryByText("No user defined yet");
const noUserInfo = await screen.queryByText("No user defined yet.");
expect(noUserInfo).toBeNull();
screen.getByText("YaST Team Member");
screen.getByText("ytm");
Expand Down
8 changes: 6 additions & 2 deletions web/src/components/users/RootAuthMethods.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,12 @@ import { useInstallerClient } from "~/context/installer";
const MethodsNotDefined = ({ setPassword, setSSHKey }) => {
return (
<div className="stack">
<div className="bold">{_("No root authentication method defined yet")}</div>
<div>{_("Please, define at least one authentication method for logging into the system as root.")}</div>
<div>{_("No root authentication method defined yet.")}</div>
<div>
<strong>
{_("Please, define at least one authentication method for logging into the system as root.")}
</strong>
</div>
<div className="split">
{/* TRANSLATORS: push button label */}
<Button variant="primary" onClick={setPassword}>{_("Set a password")}</Button>
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/users/RootAuthMethods.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ describe("when ready", () => {
it("renders a text inviting the user to define at least one", async () => {
installerRender(<RootAuthMethods />);

await screen.findByText("No root authentication method defined yet");
await screen.findByText("No root authentication method defined yet.");
screen.getByText(/at least one/);
});

Expand Down

0 comments on commit 5e882f8

Please sign in to comment.