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.

Sign up for GitHub

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

Jump to bottom

Fix DOM nesting on the Storage page dialogs, enable DOM nesting errors #19509

Merged
merged 5 commits into from
Oct 26, 2023
Merged

Fix DOM nesting on the Storage page dialogs, enable DOM nesting errors #19509

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
12d9280
storaged: Fix nesting in StopProcessesMessage dialog
martinpitt Oct 24, 2023
cb62e29
storaged: Fix DOM nesting in UsersPopover
martinpitt Oct 25, 2023
27c5e7f
storaged: Fix nested Form
martinpitt Oct 24, 2023
5d835e5
storaged: Fix nesting in tang key verification dialog
martinpitt Oct 24, 2023
c71e152
test: Fail on React DOM nesting errors
martinpitt Oct 20, 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
46 changes: 21 additions & 25 deletions pkg/storaged/crypto-keyslots.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import React from "react";
import { Card, CardBody, CardHeader, CardTitle } from '@patternfly/react-core/dist/esm/components/Card/index.js';
import { Checkbox } from "@patternfly/react-core/dist/esm/components/Checkbox/index.js";
import { ClipboardCopy } from "@patternfly/react-core/dist/esm/components/ClipboardCopy/index.js";
import { Form, FormGroup } from "@patternfly/react-core/dist/esm/components/Form/index.js";
import { FormGroup } from "@patternfly/react-core/dist/esm/components/Form/index.js";
import { DataList, DataListCell, DataListItem, DataListItemCells, DataListItemRow } from "@patternfly/react-core/dist/esm/components/DataList/index.js";
import { Text, TextContent, TextList, TextListItem, TextVariants } from "@patternfly/react-core/dist/esm/components/Text/index.js";
import { TextInput as TextInputPF } from "@patternfly/react-core/dist/esm/components/TextInput/index.js";
Expand Down Expand Up @@ -622,15 +622,13 @@ export const TangKeyVerification = ({ url, adv }) => {
<Text component={TextVariants.p}>{_("Check the key hash with the Tang server.")}</Text>

<Text component={TextVariants.h3}>{_("How to check")}</Text>
<Text component={TextVariants.p}>
{_("In a terminal, run: ")}
<ClipboardCopy hoverTip={_("Copy to clipboard")}
clickTip={_("Successfully copied to clipboard!")}
variant="inline-compact"
mvollmer marked this conversation as resolved.
Show resolved Hide resolved
isCode>
{cmd}
</ClipboardCopy>
</Text>
<span>{_("In a terminal, run: ")}</span>
<ClipboardCopy hoverTip={_("Copy to clipboard")}
clickTip={_("Successfully copied to clipboard!")}
variant="inline-compact"
isCode>
{cmd}
</ClipboardCopy>
<Text component={TextVariants.p}>
{_("Check that the SHA-256 or SHA-1 hash from the command matches this dialog.")}
</Text>
Expand Down Expand Up @@ -692,21 +690,19 @@ const RemovePassphraseField = (tag, key, dev) => {
return (
<Stack hasGutter>
<p>{ fmt_to_fragments(_("Passphrase removal may prevent unlocking $0."), <b>{dev}</b>) }</p>
<Form>
<Checkbox id="force-remove-passphrase"
isChecked={val !== false}
label={_("Confirm removal with an alternate passphrase")}
onChange={(_event, checked) => change(checked ? "" : false)}
body={val === false
? <p className="slot-warning">
{_("Removing a passphrase without confirmation of another passphrase may prevent unlocking or key management, if other passphrases are forgotten or lost.")}
</p>
: <FormGroup label={_("Passphrase from any other key slot")} fieldId="remove-passphrase">
<TextInputPF id="remove-passphrase" type="password" value={val} onChange={(_event, value) => change(value)} />
</FormGroup>
}
/>
</Form>
<Checkbox id="force-remove-passphrase"
isChecked={val !== false}
label={_("Confirm removal with an alternate passphrase")}
onChange={(_event, checked) => change(checked ? "" : false)}
Copy link
Contributor

Choose a reason for hiding this comment

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

This added line is not executed by any test. Details

body={val === false
? <p className="slot-warning">
{_("Removing a passphrase without confirmation of another passphrase may prevent unlocking or key management, if other passphrases are forgotten or lost.")}
</p>
: <FormGroup label={_("Passphrase from any other key slot")} fieldId="remove-passphrase">
<TextInputPF id="remove-passphrase" type="password" value={val} onChange={(_event, value) => change(value)} />
</FormGroup>
}
/>
</Stack>
);
}
Expand Down
22 changes: 12 additions & 10 deletions pkg/storaged/dialog.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1107,27 +1107,27 @@ const UsersPopover = ({ users }) => {
bodyContent={
<>
{ services.length > 0
? <p>
<b>{_("Services using the location")}</b>
? <>
<p><b>{_("Services using the location")}</b></p>
<List>
{ services.slice(0, max).map((u, i) => <ListItem key={i}>{u.unit.replace(/\.service$/, "")}</ListItem>) }
{ services.length > max ? <ListItem key={max}>...</ListItem> : null }
</List>
</p>
</>
: null
}
{ services.length > 0 && processes.length > 0
? <br />
: null
}
{ processes.length > 0
? <p>
<b>{_("Processes using the location")}</b>
? <>
<p><b>{_("Processes using the location")}</b></p>
<List>
{ processes.slice(0, max).map((u, i) => <ListItem key={i}>{u.comm} (user: {u.user}, pid: {u.pid})</ListItem>) }
{ processes.length > max ? <ListItem key={max}>...</ListItem> : null }
</List>
</p>
</>
: null
}
</>}>
Expand Down Expand Up @@ -1257,7 +1257,8 @@ export const StopProcessesMessage = ({ mount_point, users }) => {
return (
<div className="modal-footer-teardown">
{ process_rows.length > 0
? <p>{fmt_to_fragments(_("The mount point $0 is in use by these processes:"), <b>{mount_point}</b>)}
? <>
<p>{fmt_to_fragments(_("The mount point $0 is in use by these processes:"), <b>{mount_point}</b>)}</p>
<ListingTable variant='compact'
columns={
[
Expand All @@ -1268,15 +1269,16 @@ export const StopProcessesMessage = ({ mount_point, users }) => {
]
}
rows={process_rows} />
</p>
</>
: null
}
{ process_rows.length > 0 && service_rows.length > 0
? <br />
: null
}
{ service_rows.length > 0
? <p>{fmt_to_fragments(_("The mount point $0 is in use by these services:"), <b>{mount_point}</b>)}
? <>
<p>{fmt_to_fragments(_("The mount point $0 is in use by these services:"), <b>{mount_point}</b>)}</p>
<ListingTable variant='compact'
columns={
[
Expand All @@ -1287,7 +1289,7 @@ export const StopProcessesMessage = ({ mount_point, users }) => {
]
}
rows={service_rows} />
</p>
</>
: null
}
</div>);
Expand Down
2 changes: 0 additions & 2 deletions test/common/testlib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1701,8 +1701,6 @@ def login_and_go(self, path: Optional[str] = None, user: Optional[str] = None, h

# List of allowed console.error() messages during tests; these match substrings
default_allowed_console_errors = [
# HACK: Fix these ASAP, these are major bugs
"Warning: validateDOMNesting.*cannot appear as a",
# HACK: These should be fixed, but debugging these is not trivial, and the impact is very low
"Warning: .* setState.*on an unmounted component",
"Warning: Can't perform a React state update on an unmounted component",
Expand Down
3 changes: 3 additions & 0 deletions test/verify/check-storage-luks
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,9 @@ class TestStorageLuks(storagelib.StorageCase):
b.logout()
testlib.wait(lambda: m.execute("(loginctl list-users | grep admin) || true") == "")

# FIXME: race condition after unmounting; hard to investigate, re-check after storage redesign
self.allow_browser_errors("validateDOMNesting.*cannot appear as a child.*<tr> ul")

def testNoFsys(self):
m = self.machine
b = self.browser
Expand Down
6 changes: 6 additions & 0 deletions test/verify/check-storage-lvm2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,9 @@ class TestStorageLvm2(storagelib.StorageCase):
b.wait_not_in_text("#devices", "vgroup1")
self.assertEqual(m.execute(f"grep {mount_point_thin} /etc/fstab || true"), "")

# FIXME: Grow/Shrink buttons are in a dd > div > dd without dl wrapper
self.allow_browser_errors("validateDOMNesting.*cannot appear as a descendant.*<dd> dd")

def testUnpartitionedSpace(self):
m = self.machine
b = self.browser
Expand Down Expand Up @@ -334,6 +337,9 @@ class TestStorageLvm2(storagelib.StorageCase):
self.content_row_wait_in_col(1, 1, "lvol0")
b.wait_not_in_text("#storage-detail", "snap0")

# FIXME: Grow/Shrink buttons are in a dd > div > dd without dl wrapper
self.allow_browser_errors("validateDOMNesting.*cannot appear as a descendant.*<dd> dd")


if __name__ == '__main__':
testlib.test_main()
2 changes: 1 addition & 1 deletion test/verify/check-storage-used
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ ExecStart=/usr/bin/sleep infinity
b.wait_in_text(".pf-v5-c-popover", str(sleep_pid))
b.wait_in_text(".pf-v5-c-popover", "keep-mnt-busy")
b.assert_pixels(".pf-v5-c-popover", "popover",
mock={".pf-v5-c-popover__body p:nth-of-type(2) li": "process (user: root, pid: 1234)"},
mock={".pf-v5-c-popover__body ul:nth-of-type(2) li": "process (user: root, pid: 1234)"},
scroll_into_view="#dialog button:contains(Currently in use)")
b.click(".pf-v5-c-popover button")
b.assert_pixels('#dialog', "format", wait_after_layout_change=True)
Expand Down