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

[web] Change the location of page options #545

Merged
merged 23 commits into from
Apr 25, 2023
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1daf2a9
[web] Rename the layout slot for app actions
dgdavid Apr 21, 2023
c005251
[web] Add new layout slot for contextual/page actions
dgdavid Apr 21, 2023
27715e8
[web] Add a new ContextualActions core component
dgdavid Apr 21, 2023
67ce72c
[web] Remove header no longer needed
dgdavid Apr 21, 2023
1d2f5ef
[web] Adapt ProposalPage for using ContextualActions
dgdavid Apr 21, 2023
834ffc8
[web] Move ProposalPage options to its own file
dgdavid Apr 21, 2023
1e46837
[web] Adapt NetworkPage for using ContextualActions
dgdavid Apr 21, 2023
e403deb
[web] Fix broken test
dgdavid Apr 21, 2023
1e79a2d
[web] Display the change product link always
dgdavid Apr 21, 2023
dd0065b
[web] Drop the PageOptions layout slot
dgdavid Apr 21, 2023
3145b0a
[web] Drop PageOptions component
dgdavid Apr 21, 2023
dcdf6ba
[web] CSS adjustment
dgdavid Apr 21, 2023
1cb6b59
[web] Internal improvements for NetworkPage
dgdavid Apr 21, 2023
e5e9ec0
[web] Do not import no longer used test util
dgdavid Apr 21, 2023
e91e7cc
[web] Limit when connect to WiFi menu option is rendered
dgdavid Apr 24, 2023
d80381b
[web] Delete a no longer needed nor true hint
dgdavid Apr 24, 2023
a492944
[web] Drop Sidebar.OpenButton component
dgdavid Apr 24, 2023
390329e
[web] Add documentation and test for ContextualActions
dgdavid Apr 24, 2023
a195e8d
[web] CSS adjustment
dgdavid Apr 24, 2023
6a89990
[web] Rename ContextualActions to PageOptions
dgdavid Apr 24, 2023
fd4f079
[web] Add documentation
dgdavid Apr 24, 2023
a60b512
[web] Update the changes file
dgdavid Apr 24, 2023
6d9c61b
[web] Please linters
dgdavid Apr 25, 2023
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
22 changes: 20 additions & 2 deletions web/src/assets/styles/blocks.scss
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Standard section
// In the future we might need to use an specific CSS class for it if we start having different
// section layouts.
section {
section:not([class^="pf-c"]) {
display: grid;
grid-template-columns: var(--icon-size-m) 1fr;
grid-template-areas:
Expand All @@ -10,7 +10,7 @@ section {
gap: var(--spacer-small);
}

section:not(:last-child) {
section:not(:last-child, [class^="pf-c"]) {
margin-block-end: var(--spacer-large);
}

Expand Down Expand Up @@ -196,3 +196,21 @@ section > .content {
--pf-c-progress--GridGap: var(--spacer-small);
}
}

.contextual-actions a {
font-size: 16px;
font-weight: var(--fw-bold);
text-decoration: none;

svg {
color: inherit;
}

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

svg {
color: var(--color-link);
}
}
}
73 changes: 73 additions & 0 deletions web/src/components/core/ContextualActions.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* Copyright (c) [2023] SUSE LLC
*
* All Rights Reserved.
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of version 2 of the GNU General Public License as published
* by the Free Software Foundation.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program; if not, contact SUSE LLC.
*
* To contact SUSE LLC about this file by physical or electronic mail, you may
* find current contact information at www.suse.com.
*/

import React, { useState } from 'react';
import { Button, Dropdown, DropdownItem, DropdownGroup } from '@patternfly/react-core';
import { Icon, ContextualActions as ContextualActionsSlot } from "~/components/layout";
dgdavid marked this conversation as resolved.
Show resolved Hide resolved

const Toggler = ({ onClick }) => {
return (
<Button onClick={onClick} variant="plain">
<Icon name="expand_more" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I actually like the "+" icon, which for me has a lot of sense (more page actions). But since I thought it might be a bit controversial... I opted for using a "dropdown arrow" like one. Anyway, see how they look side by side

"Expand more" icon "Add" icon
Storage page using "expand more" icon Storage page using "add" icon

Either way, there are plenty of them at https://fonts.google.com/icons. Have a look to see if you find one that fits better.

Copy link
Contributor

Choose a reason for hiding this comment

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

I like both, and I see the down arrow more generic. I would it for now.

Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer arrow instead of + as + invokes to me adding something like new storage. And it is not generic to all usage and we should be really be consistent, so user when get familiar with one page will be familiar with all of them.

Copy link
Contributor

Choose a reason for hiding this comment

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

As said, I also like the arrow. But I am asking myself if the icon is dicoverable enough. At first sight it looks to me like a button for minimizing or something similar (maybe it is my fault as user). Moreover, how can we refer to it? I mean, in the devices selector popup we have a sentence for suggesting the user to configure more devices if needed. Do we need to keep that tip? If so, how?

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, how do you see to add a button? E.g., "More options V", with good styles to integrate it nicely with the sidebar.

Copy link
Contributor Author

@dgdavid dgdavid Apr 25, 2023

Choose a reason for hiding this comment

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

I do not think it is needed such verbosity. But if you want, we can go ahead and fill the header with words 😃

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'd use the tooltip here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At first sight it looks to me like a button for minimizing or something similar

That's one of the reason why I prefer another icon. But I do not think, honestly, this would be a big issue at this time. What is more, I'd like to have feedback for real users. I.e., start simple and react to the audience feedback. The worst can happen is to have a user discovering more options becuase wanted to minimize the storage page.

</Button>
);
};

const Group = ({ children, ...props }) => {
return (
<DropdownGroup {...props}>
{children}
</DropdownGroup>
);
};

const Item = ({ children, ...props }) => {
return (
<DropdownItem {...props}>
{children}
</DropdownItem>
);
};

const ContextualActions = ({ children }) => {
const [isOpen, setIsOpen] = useState(false);
const onToggle = () => setIsOpen(!isOpen);
const onSelect = () => setIsOpen(false);

return (
<ContextualActionsSlot>
<Dropdown
isOpen={isOpen}
toggle={<Toggler onClick={onToggle} />}
onSelect={onSelect}
dropdownItems={Array(children)}
position="right"
className="contextual-actions"
isGrouped
/>
</ContextualActionsSlot>
);
};

ContextualActions.Group = Group;
ContextualActions.Item = Item;

export default ContextualActions;
82 changes: 0 additions & 82 deletions web/src/components/core/PageOptions.jsx

This file was deleted.

71 changes: 0 additions & 71 deletions web/src/components/core/PageOptions.test.jsx

This file was deleted.

6 changes: 3 additions & 3 deletions web/src/components/core/Sidebar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

import React, { useEffect, useRef, useState } from "react";
import { Button, Text } from "@patternfly/react-core";
import { Icon, PageActions } from "~/components/layout";
import { Icon, AppActions } from "~/components/layout";

// FIXME: look for a better way to allow opening the Sidebar from outside
let openButtonRef = {};
Expand Down Expand Up @@ -89,7 +89,7 @@ const Sidebar = ({ children }) => {

return (
<>
<PageActions>
<AppActions>
<button
ref={openButtonRef}
onClick={open}
Expand All @@ -100,7 +100,7 @@ const Sidebar = ({ children }) => {
>
<Icon name="menu" />
</button>
</PageActions>
</AppActions>

<nav
id="navigation-and-options"
Expand Down
28 changes: 14 additions & 14 deletions web/src/components/core/Sidebar.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,10 @@

import React from "react";
import { screen, within } from "@testing-library/react";
import { plainRender, mockComponent, mockLayout } from "~/test-utils";
import { PageOptions, Sidebar } from "~/components/core";
import { plainRender, mockLayout } from "~/test-utils";
import { Sidebar } from "~/components/core";

jest.mock("~/components/layout/Layout", () => mockLayout());
jest.mock("~/components/core/PageOptions", () => mockComponent(
<>
<a href="#">Goes somewhere</a>
<a href="#" data-keep-sidebar-open="true">Keep it open!</a>
<button>Do something</button>
<button data-keep-sidebar-open="true">Keep it open!</button>
</>
));

it("renders the sidebar initially hidden", async () => {
plainRender(<Sidebar />);
Expand All @@ -41,7 +33,7 @@ it("renders the sidebar initially hidden", async () => {
});

it("renders a link for displaying the sidebar", async () => {
const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
const { user } = plainRender(<Sidebar />);

const link = await screen.findByLabelText(/Show/i);
const nav = await screen.findByRole("navigation", { name: /options/i });
Expand All @@ -52,7 +44,7 @@ it("renders a link for displaying the sidebar", async () => {
});

it("renders a link for hiding the sidebar", async () => {
const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
const { user } = plainRender(<Sidebar />);

const openLink = await screen.findByLabelText(/Show/i);
const closeLink = await screen.findByLabelText(/Hide/i);
Expand All @@ -66,7 +58,7 @@ it("renders a link for hiding the sidebar", async () => {
});

it("moves the focus to the close action after opening it", async () => {
const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
const { user } = plainRender(<Sidebar />);

const openLink = await screen.findByLabelText(/Show/i);
const closeLink = await screen.findByLabelText(/Hide/i);
Expand All @@ -78,7 +70,15 @@ it("moves the focus to the close action after opening it", async () => {

describe("onClick bubbling", () => {
it("hides the sidebar only if the user clicked on a link or button w/o keepSidebarOpen attribute", async () => {
const { user } = plainRender(<Sidebar><PageOptions /></Sidebar>);
const { user } = plainRender(
<Sidebar>
<a href="#">Goes somewhere</a>
<a href="#" data-keep-sidebar-open="true">Keep it open!</a>
<button>Do something</button>
<button data-keep-sidebar-open="true">Keep it open!</button>
</Sidebar>
);

const openLink = screen.getByLabelText(/Show/i);
await user.click(openLink);
const nav = screen.getByRole("navigation", { name: /options/i });
Expand Down
2 changes: 1 addition & 1 deletion web/src/components/core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/

export { default as About } from "./About";
export { default as ContextualActions } from "./ContextualActions";
export { default as Disclosure } from "./Disclosure";
export { default as Sidebar } from "./Sidebar";
export { default as Section } from "./Section";
Expand All @@ -39,7 +40,6 @@ export { default as FileViewer } from "./FileViewer";
export { default as RowActions } from "./RowActions";
export { default as ShowLogButton } from "./ShowLogButton";
export { default as Page } from "./Page";
export { default as PageOptions } from "./PageOptions";
export { default as PasswordAndConfirmationInput } from "./PasswordAndConfirmationInput";
export { default as Popup } from "./Popup";
export { default as ProgressReport } from "./ProgressReport";
Expand Down
2 changes: 2 additions & 0 deletions web/src/components/layout/Icon.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import Downloading from "@icons/downloading.svg?component";
import Edit from "@icons/edit.svg?component";
import EditSquare from "@icons/edit_square.svg?component";
import Error from "@icons/error.svg?component";
import ExpandMore from "@icons/expand_more.svg?component";
import HardDrive from "@icons/hard_drive.svg?component";
import Help from "@icons/help.svg?component";
import HomeStorage from "@icons/home_storage.svg?component";
Expand Down Expand Up @@ -78,6 +79,7 @@ const icons = {
edit: Edit,
edit_square: EditSquare,
error: Error,
expand_more: ExpandMore,
hard_drive: HardDrive,
help: Help,
home_storage: HomeStorage,
Expand Down
Loading