Skip to content

Commit

Permalink
Merge pull request #877 from openSUSE/fix-storage-menuitems
Browse files Browse the repository at this point in the history
[web] Fix broken storage links

During [migration to PF5](#759), specifically when [moving to the new PF/Dropdown](2edc4e2#diff-88232d7f99420122a9e8c24ae183bcf9a9b98451cbb5db44dea461a2d5ed6727), a few options were unintentionally broken when their `href` prop were not properly renamed to `to`.

This [makes the PF/MenuItem to use a `<button>` instead of `<a>`](https://github.com/patternfly/patternfly-react/blob/7d2fd2678f993459f67a04d173a8d16c16ef50a3/packages/react-core/src/components/Menu/MenuItem.tsx#L139) inside an `<li>` when building the menu item, assigning the `href` HTML attribute to the latest, which has no effect at all.

As a result, users cannot navigate to these options through the UI, as it was the case in https://bugzilla.suse.com/show_bug.cgi?id=1217281
  • Loading branch information
dgdavid authored Nov 17, 2023
2 parents 7785807 + 3b418b0 commit c9784cf
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 7 deletions.
5 changes: 5 additions & 0 deletions web/package/cockpit-agama.changes
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
-------------------------------------------------------------------
Fri Nov 17 13:27:22 UTC 2023 - David Diaz <dgonzalez@suse.com>

- UI: Fix broken storage links (bsc#1217281).

-------------------------------------------------------------------
Wed Nov 15 12:32:25 UTC 2023 - José Iván López González <jlopez@suse.com>

Expand Down
4 changes: 2 additions & 2 deletions web/src/components/core/PageOptions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,14 @@ const Options = ({ children, ...props }) => {
* <PageOptions.Options>
* <PageOptions.Option
* key="dasd-link"
* href={href}
* to={href}
* description="Manage and format"
* >
* DASD
* </PageOptions.Option>
* <PageOptions.Option
* key="iscsi-link"
* href={href}
* to={href}
* description="Connect to iSCSI targets"
* >
* iSCSI
Expand Down
4 changes: 2 additions & 2 deletions web/src/components/storage/ProposalPageOptions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const DASDLink = () => {
return (
<PageOptions.Option
key="dasd-link"
href={href}
to={href}
description={_("Manage and format")}
>
DASD
Expand All @@ -54,7 +54,7 @@ const ZFCPLink = () => {
return (
<PageOptions.Option
key="zfcp-link"
href={href}
to={href}
description={_("Activate disks")}
>
{_("zFCP")}
Expand Down
9 changes: 6 additions & 3 deletions web/src/components/storage/ProposalPageOptions.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,17 @@ it("contains an entry for configuring iSCSI", async () => {
const { user } = installerRender(<ProposalPageOptions />);
const toggler = screen.getByRole("button");
await user.click(toggler);
screen.getByRole("menuitem", { name: /iSCSI/ });
const link = screen.getByRole("menuitem", { name: /iSCSI/ });
expect(link).toHaveAttribute("href", "/storage/iscsi");
});

it("contains an entry for configuring DASD when is supported", async () => {
isDASDSupportedFn.mockResolvedValue(true);
const { user } = installerRender(<ProposalPageOptions />);
const toggler = screen.getByRole("button");
await user.click(toggler);
screen.getByRole("menuitem", { name: /DASD/ });
const link = screen.getByRole("menuitem", { name: /DASD/ });
expect(link).toHaveAttribute("href", "/storage/dasd");
});

it("does not contain an entry for configuring DASD when is NOT supported", async () => {
Expand All @@ -78,7 +80,8 @@ it("contains an entry for configuring zFCP when is supported", async () => {
const { user } = installerRender(<ProposalPageOptions />);
const toggler = screen.getByRole("button");
await user.click(toggler);
screen.getByRole("menuitem", { name: /zFCP/ });
const link = screen.getByRole("menuitem", { name: /zFCP/ });
expect(link).toHaveAttribute("href", "/storage/zfcp");
});

it("does not contain an entry for configuring zFCP when is NOT supported", async () => {
Expand Down

0 comments on commit c9784cf

Please sign in to comment.