Skip to content

Commit

Permalink
refactor(web): UI code pruning and clean up round #3 (#1540)
Browse files Browse the repository at this point in the history
**Apart from a bit of clean up, this PR is intended for start writing
better core components** that has been on hold for a few months already.

It's the case of _core/Page_ component, which has been rewritten almost
for scratch and now makes the weird _core/CardField transitioning
component_ obsolete.


Please, note that this set of changes **continues with the migration to
TypeScript for touched files** and also **introduce a PatternFly/Flex
wrapper** in order to ease the work with its responsive props. It's a
bit complex because the (ab)use of advanced types but it does the job
without introducing props unknown by PF/Flex. As said in the file
comments, ideally

> would be better to add these responsive props shortcuts direclty in
PF/Flex to allow the consumer to just set the `default` value when not
needed to change it depending on the breakpoint. But at this moment
we're a bit short of time for creating and testing such an elaborated PR
against upstream.

---

Related to #1441 and
#1494
  • Loading branch information
dgdavid authored Sep 13, 2024
2 parents 04bd65a + 1dfad27 commit 9a6d145
Show file tree
Hide file tree
Showing 47 changed files with 1,486 additions and 1,540 deletions.
204 changes: 0 additions & 204 deletions web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,44 +2,13 @@
// In the future we might add different section layouts by using data-variant attribute
// or similar strategy

// raw file content with formatting similar to <pre>
.filecontent {
font-family: var(--ff-code);
font-size: 90%;
word-break: break-all;
white-space: pre-wrap;
}

// Make progress more compact
.dasd-format-progress {
.pf-v5-c-progress {
--pf-v5-c-progress--GridGap: var(--spacer-small);
}
}

[data-type="agama/page-menu"] {
> button {
--pf-v5-c-button--PaddingRight: 0;
}

a {
font-weight: var(--fw-bold);
text-decoration: none;

svg {
color: inherit;
}

&:hover {
color: var(--color-link-hover);

svg {
color: var(--color-link);
}
}
}
}

.issue {
--icon-size: 1rem;

Expand All @@ -55,174 +24,6 @@
}
}

ul[data-type="agama/list"] {
list-style: none;
margin-inline: 0;

li {
border: 2px solid var(--color-gray-dark);
padding: var(--spacer-small);
text-align: start;
background: var(--color-gray-light);
margin-block-end: 0;

&:nth-child(n + 2) {
border-top: 0;
}

&:not(:last-child) {
border-bottom-width: 1px;
}

> div {
margin-block-end: var(--spacer-smaller);
}

// Done in two rules instead of div:not(:last-child) to avoid specificity
// problems later; see the storage-devices selector
> div:last-child {
margin-block-end: 0;
}
}

// FIXME: see if it's semantically correct to mark an li as aria-selected when
// not belongs to a listbox or grid list ul.
li[aria-selected] {
background: var(--color-gray-dark);

&:not(:last-child) {
border-bottom-color: white;
}
}
}

// These attributes together means that UI is rendering a selector
ul[data-type="agama/list"][role="grid"] {
li[role="row"] {
cursor: pointer;

&:first-child {
border-radius: 5px 5px 0 0;
}

&:last-child {
border-radius: 0 0 5px 5px;
}

&:only-child {
border-radius: 5px;
}

&:hover {
&:not([aria-selected]) {
background: var(--color-gray-dark);
}

&:not(:last-child) {
border-bottom-color: white;
}
}

div[role="gridcell"] {
display: flex;
align-items: center;
gap: var(--spacer-small);

input {
--size: var(--fs-h2);
cursor: pointer;
block-size: var(--size);
inline-size: var(--size);

&[data-auto-selected] {
accent-color: white;
box-shadow: 0 0 1px;
}
}

& > div:first-child {
display: flex;
flex-direction: column;
align-items: center;
gap: var(--spacer-small);

span {
font-size: var(--fs-small);
font-weight: bold;
}
}

& > div:last-child {
flex: 1;
}
}
}
}

[data-items-type="agama/space-policies"] {
// It works with the default styling
}

[data-items-type="agama/locales"] {
display: grid;
grid-template-columns: 1fr 2fr;

> :last-child {
grid-column: 1 / -1;
font-size: var(--fs-small);
}
}

[data-items-type="agama/keymaps"] {
> :last-child {
font-size: var(--fs-small);
}
}

[data-items-type="agama/timezones"] {
display: grid;
grid-template-columns: 2fr 1fr 1fr;

> :last-child {
grid-column: 1 / -1;
font-size: 80%;
}

> :nth-child(3) {
color: var(--color-gray-dimmed);
text-align: end;
}
}

ul[data-items-type="agama/patterns"] {
div[role="gridcell"] {
& > div:first-child {
min-width: 65px;
}

& > div:last-child * {
margin-block-end: var(--spacer-small);
}
}
}

[role="dialog"] {
.sticky-top-0 {
position: sticky;
top: calc(-1 * var(--pf-v5-c-modal-box__body--PaddingTop));
margin-block-start: calc(-1 * var(--pf-v5-c-modal-box__body--PaddingTop));
padding-block-start: var(--pf-v5-c-modal-box__body--PaddingTop);
background-color: var(--pf-v5-c-modal-box--BackgroundColor);

[role="search"] {
width: 100%;
padding: var(--spacer-small);
border: 1px solid var(--color-primary);
border-radius: 5px;
}
}
}

table[data-type="agama/tree-table"] {
th:first-child {
padding-inline-end: var(--spacer-normal);
Expand Down Expand Up @@ -342,11 +143,6 @@ table.proposal-result {
}
}

.highlighted-live-region {
padding: 10px;
background: var(--color-gray);
}

.size-input-group {
max-inline-size: 20ch;

Expand Down
86 changes: 0 additions & 86 deletions web/src/components/core/CardField.jsx

This file was deleted.

4 changes: 2 additions & 2 deletions web/src/components/core/ListSearch.jsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) [2023] SUSE LLC
* Copyright (c) [2023-2024] SUSE LLC
*
* All Rights Reserved.
*
Expand Down Expand Up @@ -42,7 +42,7 @@ const search = (elements, term) => {
* @param {object} props
* @param {string} [props.placeholder]
* @param {object[]} [props.elements] - List of elements in which to search.
* @param {(elements: object[]) => void} - Callback to be called with the filtered list of elements.
* @param {(elements: object[]) => void} [props.onChange] - Callback to be called with the filtered list of elements.
*/
export default function ListSearch({
placeholder = _("Search"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ import { plainRender } from "~/test-utils";
import { LoginPage } from "~/components/core";
import { AuthErrors } from "~/context/auth";

let mockIsAuthenticated;
const mockLoginFn = jest.fn();
let consoleErrorSpy: jest.SpyInstance;
let mockIsAuthenticated: boolean;
let mockLoginError;
const mockLoginFn = jest.fn();

jest.mock("~/context/auth", () => ({
...jest.requireActual("~/context/auth"),
Expand All @@ -40,16 +41,16 @@ jest.mock("~/context/auth", () => ({
},
}));

describe.skip("LoginPage", () => {
describe("LoginPage", () => {
beforeAll(() => {
mockIsAuthenticated = false;
mockLoginError = null;
mockLoginFn.mockResolvedValue({ status: 200 });
jest.spyOn(console, "error").mockImplementation();
consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
});

afterAll(() => {
console.error.mockRestore();
consoleErrorSpy.mockRestore();
});

describe("when user is not authenticated", () => {
Expand Down Expand Up @@ -114,7 +115,7 @@ describe.skip("LoginPage", () => {

it("renders a button to know more about the project", async () => {
const { user } = plainRender(<LoginPage />);
const button = screen.getByRole("button", { name: "What is this?" });
const button = screen.getByRole("button", { name: "More about this" });

await user.click(button);

Expand Down
Loading

0 comments on commit 9a6d145

Please sign in to comment.