Skip to content

Commit

Permalink
webui: port disk selector to the new Select implementation
Browse files Browse the repository at this point in the history 
As noticed the new Select implementation from PF5 is way more
configurable than the old one but for code readability purposed and
developer experience it is a regression in comparison with the
Patternfly 4 component. [1]

[1] Opened an issue about this upstream: patternfly/patternfly-react#9511
  • Loading branch information
@KKoukiou
KKoukiou committed Aug 17, 2023
1 parent 99630dd commit 2e1b566
Show file tree
Hide file tree
Showing 3 changed files with 218 additions and 74 deletions.
272 changes: 208 additions & 64 deletions ui/webui/src/components/storage/InstallationMethod.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,18 +21,22 @@ import {
Alert,
AlertActionCloseButton,
Button,
Chip,
ChipGroup,
Flex,
FlexItem,
Form,
FormGroup,
Title
} from "@patternfly/react-core";
import {
MenuToggle,
Select,
SelectList,
SelectOption,
SelectVariant,
} from "@patternfly/react-core/deprecated";
import { SyncAltIcon, WrenchIcon } from "@patternfly/react-icons";
TextInputGroup,
TextInputGroupMain,
TextInputGroupUtilities,
Title,
} from "@patternfly/react-core";
import { SyncAltIcon, TimesIcon, WrenchIcon } from "@patternfly/react-icons";

import { InstallationScenario } from "./InstallationScenario.jsx";

Expand Down Expand Up @@ -86,10 +90,196 @@ const containEqualDisks = (disks1, disks2) => {
return disks1Str === disks2Str;
};

const LocalDisksSelect = ({ deviceData, diskSelection, idPrefix, setSelectedDisks }) => {
const [isOpen, setIsOpen] = useState(false);
const [inputValue, setInputValue] = useState("");
const [focusedItemIndex, setFocusedItemIndex] = useState(null);
const textInputRef = useRef();

let selectOptions = diskSelection.usableDisks
.map(disk => ({
description: deviceData[disk]?.description.v,
name: disk,
size: cockpit.format_bytes(deviceData[disk]?.total.v),
value: disk,
}))
.filter(option =>
String(option.name)
.toLowerCase()
.includes(inputValue.toLowerCase()) ||
String(option.description)
.toLowerCase()
.includes(inputValue.toLowerCase())
);

if (selectOptions.length === 0) {
selectOptions = [
{ children: _("No results found"), value: "no results" }
];
}

const onSelect = (selectedDisk) => {
if (diskSelection.selectedDisks.includes(selectedDisk)) {
setSelectedDisks({ drives: diskSelection.selectedDisks.filter(disk => disk !== selectedDisk) });
} else {
setSelectedDisks({ drives: [...diskSelection.selectedDisks, selectedDisk] });
}
textInputRef.current?.focus();
};

const clearSelection = () => {
setSelectedDisks({ drives: [] });
};

const handleMenuArrowKeys = (key) => {
let indexToFocus;

if (isOpen) {
if (key === "ArrowUp") {
// When no index is set or at the first index, focus to the last, otherwise decrement focus index
if (focusedItemIndex === null || focusedItemIndex === 0) {
indexToFocus = selectOptions.length - 1;
} else {
indexToFocus = focusedItemIndex - 1;
}
}

if (key === "ArrowDown") {
// When no index is set or at the last index, focus to the first, otherwise increment focus index
if (focusedItemIndex === null || focusedItemIndex === selectOptions.length - 1) {
indexToFocus = 0;
} else {
indexToFocus = focusedItemIndex + 1;
}
}

setFocusedItemIndex(indexToFocus);
}
};

const onInputKeyDown = (event) => {
const enabledMenuItems = selectOptions.filter((menuItem) => !menuItem.isDisabled);
const [firstMenuItem] = enabledMenuItems;
const focusedItem = focusedItemIndex ? enabledMenuItems[focusedItemIndex] : firstMenuItem;

switch (event.key) {
// Select the first available option
case "Enter":
if (!isOpen) {
setIsOpen((prevIsOpen) => !prevIsOpen);
} else if (isOpen && focusedItem.name !== "no results") {
onSelect(focusedItem.name);
}
break;
case "Tab":
case "Escape":
setIsOpen(false);
break;
case "ArrowUp":
case "ArrowDown":
event.preventDefault();
handleMenuArrowKeys(event.key);
break;
}
};

const onToggleClick = () => {
setIsOpen(!isOpen);
};

const onTextInputChange = (_event, value) => {
setInputValue(value);
};

const toggle = (toggleRef) => (
<MenuToggle
id={idPrefix + "-toggle"}
variant="typeahead"
onClick={onToggleClick}
innerRef={toggleRef}
isExpanded={isOpen}
className={idPrefix}
>
<TextInputGroup isPlain>
<TextInputGroupMain
value={inputValue}
onClick={onToggleClick}
onChange={onTextInputChange}
onKeyDown={onInputKeyDown}
autoComplete="off"
innerRef={textInputRef}
placeholder={_("Select a disk")}
role="combobox"
isExpanded={isOpen}
>
<ChipGroup aria-label={_("Current selections")}>
{diskSelection.selectedDisks.map((selection, index) => (
<Chip
key={index}
onClick={(ev) => {
ev.stopPropagation();
onSelect(selection);
}}
>
{selection}
</Chip>
))}
</ChipGroup>
</TextInputGroupMain>
<TextInputGroupUtilities>
{diskSelection.selectedDisks.length > 0 && (
<Button
aria-label={_("Clear input value")}
id={idPrefix + "-clear"}
variant="plain"
onClick={() => {
setInputValue("");
clearSelection();
textInputRef?.current?.focus();
}}
>
<TimesIcon aria-hidden />
</Button>
)}
</TextInputGroupUtilities>
</TextInputGroup>
</MenuToggle>
);

return (
<Select
aria-labelledby={idPrefix + "-title"}
isOpen={isOpen}
onOpenChange={() => setIsOpen(false)}
onSelect={(ev, selection) => onSelect(selection)}
selected={diskSelection.selectedDisks}
toggle={toggle}
>
<SelectList isAriaMultiselectable>
{selectOptions.map((option, index) => (
<SelectOption
isDisabled={option.value === "no results"}
description={option.size}
id={idPrefix + "-option-" + option.name}
isFocused={focusedItemIndex === index}
key={option.value}
value={option.value}
>
{
option.name
? cockpit.format("$0 ($1)", option.name, option.description)
: option.children
}
</SelectOption>
))}
</SelectList>
</Select>
);
};

const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix, isBootIso, setIsFormValid, onCritFail }) => {
const [isRescanningDisks, setIsRescanningDisks] = useState(false);
const [equalDisksNotify, setEqualDisksNotify] = useState(false);
const [isOpen, setIsOpen] = useState(false);
const refUsableDisks = useRef();

debug("DiskSelector: deviceData: ", JSON.stringify(Object.keys(deviceData)), ", diskSelection: ", JSON.stringify(diskSelection));
Expand Down Expand Up @@ -162,63 +352,13 @@ const InstallationDestination = ({ deviceData, diskSelection, dispatch, idPrefix
</Button>
);

const onSelect = (event, selection) => {
const selectedDisk = selection.name;

if (diskSelection.selectedDisks.includes(selectedDisk)) {
setSelectedDisks({ drives: diskSelection.selectedDisks.filter(disk => disk !== selectedDisk) });
} else {
setSelectedDisks({ drives: [...diskSelection.selectedDisks, selectedDisk] });
}
};

const clearSelection = () => {
setSelectedDisks({ drives: [] });
};

const localDisksSelect = (
<Select
toggleId={idPrefix + "-disk-selector-toggle"}
aria-labelledby={idPrefix + "-disk-selector-title"}
variant={SelectVariant.typeaheadMulti}
onToggle={() => setIsOpen(!isOpen)}
onSelect={onSelect}
onClear={clearSelection}
selections={diskSelection.selectedDisks.map(disk => ({
toString: function () {
return `${this.description} (${this.name})`;
},
name: disk,
description: deviceData[disk]?.description.v,
compareTo: function (value) {
return this.toString()
.toLowerCase()
.includes(value.toString().toLowerCase());
}
}))}
isOpen={isOpen}
placeholderText={_("Select a disk")}
>
{diskSelection.usableDisks.map(disk => (
<SelectOption
id={idPrefix + "-disk-selector-option-" + disk}
key={disk}
value={{
toString: function () {
return `${this.description} (${this.name})`;
},
name: disk,
description: deviceData[disk]?.description.v,
compareTo: function (value) {
return this.toString()
.toLowerCase()
.includes(value.toString().toLowerCase());
}
}}
description={cockpit.format_bytes(deviceData[disk]?.total.v)}
/>
))}
</Select>
<LocalDisksSelect
idPrefix={idPrefix + "-disk-selector"}
deviceData={deviceData}
diskSelection={diskSelection}
setSelectedDisks={setSelectedDisks}
/>
);

const equalDisks = refUsableDisks.current && containEqualDisks(refUsableDisks.current, diskSelection.usableDisks);
Expand Down Expand Up @@ -293,7 +433,11 @@ export const InstallationMethod = ({
}) => {
return (
<AnacondaPage title={!isBootIso ? cockpit.format(_("Welcome. Let's install $0 now."), osRelease.REDHAT_SUPPORT_PRODUCT) : null}>
<Form className={idPrefix + "-selector"} id={idPrefix + "-selector-form"}>
<Form
className={idPrefix + "-selector"}
id={idPrefix + "-selector-form"}
onSubmit={e => { e.preventDefault(); return false }}
>
{stepNotification && (stepNotification.step === "installation-method") &&
<Alert
isInline
Expand Down
2 changes: 1 addition & 1 deletion ui/webui/src/components/storage/InstallationMethod.scss
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
.installation-method-selector {
.pf-v5-c-select {
.installation-method-disk-selector {
width: 50%;
}

Expand Down
18 changes: 9 additions & 9 deletions ui/webui/test/helpers/storage.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ def get_disks(self):

@log_step()
def select_disk(self, disk, selected=True, is_single_disk=False):
if not self.browser.is_present(f"ul[aria-labelledby='{id_prefix}-disk-selector-title']"):
self.browser.click(f"#{id_prefix}-disk-selector-toggle")
if not self.browser.is_present(f".pf-v5-c-menu[aria-labelledby='{id_prefix}-disk-selector-title']"):
self.browser.click(f"#{id_prefix}-disk-selector-toggle > button")

if selected:
self.browser.click(f"#{id_prefix}-disk-selector-option-{disk} button:not(.pf-m-selected)")
self.browser.click(f"#{id_prefix}-disk-selector-option-{disk}:not(.pf-m-selected)")
else:
self.browser.click(f"#{id_prefix}-disk-selector-option-{disk} button.pf-m-selected")
self.browser.click(f"#{id_prefix}-disk-selector-option-{disk}.pf-m-selected")

if is_single_disk:
self.check_single_disk_destination(disk)
Expand All @@ -63,7 +63,7 @@ def select_disk(self, disk, selected=True, is_single_disk=False):

@log_step()
def select_none_disks_and_check(self, disks):
self.browser.click(".pf-v5-c-select__toggle-clear")
self.browser.click(f"#{id_prefix}-disk-selector-clear")
for disk in disks:
self.check_disk_selected(disk, False)

Expand Down Expand Up @@ -172,16 +172,16 @@ def rescan_disks(self):

@log_step(snapshot_before=True)
def check_disk_visible(self, disk, visible=True):
if not self.browser.is_present(f"ul[aria-labelledby='{id_prefix}-disk-selector-title']"):
self.browser.click(f"#{id_prefix}-disk-selector-toggle")
if not self.browser.is_present(f".pf-v5-c-menu[aria-labelledby='{id_prefix}-disk-selector-title']"):
self.browser.click(f"#{id_prefix}-disk-selector-toggle > button")

if visible:
self.browser.wait_visible(f"#{id_prefix}-disk-selector-option-{disk}")
else:
self.browser.wait_not_present(f"#{id_prefix}-disk-selector-option-{disk}")

self.browser.click(f"#{id_prefix}-disk-selector-toggle")
self.browser.wait_not_present(f"ul[aria-labelledby='{id_prefix}-disk-selector-title']")
self.browser.click(f"#{id_prefix}-disk-selector-toggle > button")
self.browser.wait_not_present(f".pf-v5-c-menu[aria-labelledby='{id_prefix}-disk-selector-title']")

def _partitioning_selector(self, scenario):
return f"#{id_prefix}-scenario-" + scenario
Expand Down

0 comments on commit 2e1b566

Please sign in to comment.