[web] Storage page: break settings into multiple sections #1045

dgdavid · 2024-02-21T08:35:25Z

Break the storage settings section to accomplish the organization defined in the storage UI document. Thus, proposed changes make the page to have Device, Settings, and File systems sections instead of only Settings.

Also, it includes a last minute change about moving the LVM setting to the new Device section, because it is more

consistent with the future we want to go (in which LVM is part of the device selection)

— @ancorgs at #yast IRC channel

Before	After

Related to https://trello.com/c/SK4uBRnw (internal link)

web/src/components/storage/ProposalDeviceSection.jsx

web/src/components/storage/index.js

coveralls · 2024-02-21T08:40:51Z

coverage: 74.279% (+0.02%) from 74.255%
when pulling 75bd8aa on storage-settings-break
into e302716 on master.

And take the opportunity to avoid triggering the onChange callback when the user actually does not change the selected device but clicks the "Accept" button of the selection dialog.

And mount there the storage/ProposalVolumes component instead of doing so in the settings section. Note that, most probably, the code of ProposalFileSystemsSection and ProposalVolumes could be merged in a single component.

Intended for adding, when needed, a short explanation about its usefulness.

dgdavid · 2024-02-22T08:38:14Z

web/src/components/storage/ProposalDeviceSection.jsx

+        selected={device}
+        devices={devices}
+        onChange={changeSelected}
+      />
+    </Form>
+  );
+};
+
+/**
+ * Allows to select the installation device.
+ * @component
+ *
+ * @callback onChangeFn
+ * @param {string} device - Name of the selected device.
+ *
+ * @param {object} props
+ * @param {string} [props.current] - Device name, if any.
+ * @param {StorageDevice[]} [props.devices=[]] - Available devices for the selection.
+ * @param {boolean} [props.isLoading=false] - Whether to show the selector as loading.
+ * @param {onChangeFn} [props.onChange=noop] - On change callback.
+ */
+const InstallationDeviceField = ({
+  current,
+  devices = [],
+  isLoading = false,
+  onChange = noop
+}) => {
+  const [device, setDevice] = useState(devices.find(d => d.name === current));
+  const [isFormOpen, setIsFormOpen] = useState(false);
+
+  const openForm = () => setIsFormOpen(true);
+
+  const closeForm = () => setIsFormOpen(false);
+
+  const acceptForm = (selectedDevice) => {
+    closeForm();
+    setDevice(selectedDevice);
+    onChange(selectedDevice);
+  };
+
+  /**
+   * Renders a button that allows changing selected device
+   *
+   * NOTE: if a device is already selected, its name and size will be used for
+   * the button text. Otherwise, a "No device selected" text will be shown.
+   *
+   * @param {object} props
+   * @param {StorageDevice|undefined} [props.current] - Currently selected device, if any.
+   */
+  const DeviceContent = ({ device }) => {
+    return (
+      <Button variant="link" isInline onClick={openForm}>
+        {device ? deviceLabel(device) : _("No device selected yet")}
+      </Button>
+    );
+  };
+
+  if (isLoading) {
+    return <Skeleton screenreaderText={_("Loading selected device")} width="25%" />;
+  }
+
+  const description = _("Select the device for installing the system.");
+
+  return (
+    <>
+      <div className="split">
+        <span>{_("Installation device")}</span>
+        <DeviceContent device={device} />
+      </div>
+      <Popup
+        title={_("Installation device")}
+        description={description}
+        isOpen={isFormOpen}
+      >
+        <If
+          condition={devices.length === 0}
+          then={_("No devices found.")}
+          else={
+            <InstallationDeviceForm
+              id="bootDeviceForm"
+              current={device}
+              devices={devices}
+              onSubmit={acceptForm}
+            />
+          }
+        />
+        <Popup.Actions>
+          <Popup.Confirm
+            form="bootDeviceForm"
+            type="submit"
+            isDisabled={devices.length === 0}
+          >
+            {_("Accept")}
+          </Popup.Confirm>
+          <Popup.Cancel onClick={closeForm} />
+        </Popup.Actions>
+      </Popup>
+    </>
+  );
+};


Note: most probably, these internal components does not make sense now that the device selector it's in its own section. I.e., most of the code could be merged into the main component. But it is out of the scope of this PR, especially taking into account that there are more planned changes ahead that could affect this component too. So, let's do it in a re-factorization round.

dgdavid · 2024-02-22T08:40:16Z

web/src/components/storage/ProposalFileSystemsSection.jsx

+
+  return (
+    <Section title={_("File systems")}>
+      <ProposalVolumes


This PR does not directly integrate the <ProposalVolumes> code here because there is a plan for reworking almost the whole thing in a separated PR.

dgdavid · 2024-02-22T09:21:10Z

web/src/components/core/Section.jsx

-      <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
+      <header>
+        <h2 id={headerId}>{headerIcon}<span>{headerText}</span></h2>
+        <p>{description}</p>


By now, I'm not adding an aria-describedby to the section pointing to this. I'm not sure if

It's actually needed, since the description will be visible and I guess it will be read aloud

It will be read twice

It's needed to add an aria-hidden here too.

So, let's postpone this small detail just a bit, until we have time to test these a11y things.

@ancorgs

Based on @ancorgs's comment at #1045 (comment)

We've agreed that section title states almost the same.

To avoid having empty <p> HTML nodes when description is not given.

@ancorgs

Since, using @ancorgs's words, it is more > consistent with the future we want to go (in which LVM is part of > the device selection)

According to #1045 (comment)

web/src/components/storage/ProposalDeviceSection.jsx

web/src/components/storage/ProposalSpacePolicySection.jsx

Prepare for releasing Agama 8. It includes the following pull requests: * #884 * #886 * #914 * #918 * #956 * #957 * #958 * #959 * #960 * #961 * #962 * #963 * #964 * #965 * #966 * #969 * #970 * #976 * #977 * #978 * #979 * #980 * #981 * #983 * #984 * #985 * #986 * #988 * #991 * #992 * #995 * #996 * #997 * #999 * #1003 * #1004 * #1006 * #1007 * #1008 * #1009 * #1010 * #1011 * #1012 * #1014 * #1015 * #1016 * #1017 * #1020 * #1022 * #1023 * #1024 * #1025 * #1027 * #1028 * #1029 * #1030 * #1031 * #1032 * #1033 * #1034 * #1035 * #1036 * #1038 * #1039 * #1041 * #1042 * #1043 * #1045 * #1046 * #1047 * #1048 * #1052 * #1054 * #1056 * #1057 * #1060 * #1061 * #1062 * #1063 * #1064 * #1066 * #1067 * #1068 * #1069 * #1071 * #1072 * #1073 * #1074 * #1075 * #1079 * #1080 * #1081 * #1082 * #1085 * #1086 * #1087 * #1088 * #1089 * #1090 * #1091 * #1092 * #1093 * #1094 * #1095 * #1096 * #1097 * #1098 * #1099 * #1100 * #1102 * #1103 * #1104 * #1105 * #1106 * #1109 * #1110 * #1111 * #1112 * #1114 * #1116 * #1117 * #1118 * #1119 * #1120 * #1121 * #1122 * #1123 * #1125 * #1126 * #1127 * #1128 * #1129 * #1130 * #1131 * #1132 * #1133 * #1134 * #1135 * #1136 * #1138 * #1139 * #1140 * #1141 * #1142 * #1143 * #1144 * #1145 * #1146 * #1147 * #1148 * #1149 * #1151 * #1152 * #1153 * #1154 * #1155 * #1156 * #1157 * #1158 * #1160 * #1161 * #1162 * #1163 * #1164 * #1165 * #1166 * #1167 * #1168 * #1169 * #1170 * #1171 * #1172 * #1173 * #1174 * #1175 * #1177 * #1178 * #1180 * #1181 * #1182 * #1183 * #1184 * #1185 * #1187 * #1188 * #1189 * #1190 * #1191 * #1192 * #1193 * #1194 * #1195 * #1196 * #1198 * #1199 * #1200 * #1201 * #1203 * #1204 * #1205 * #1206 * #1207 * #1208 * #1209 * #1210 * #1211 * #1212 * #1213 * #1214 * #1215 * #1216 * #1217 * #1219 * #1220 * #1221 * #1222 * #1223 * #1224 * #1225 * #1226 * #1227 * #1229

dgdavid commented Feb 21, 2024

View reviewed changes

web/src/components/storage/ProposalDeviceSection.jsx Outdated Show resolved Hide resolved

dgdavid commented Feb 21, 2024

View reviewed changes

web/src/components/storage/index.js Show resolved Hide resolved

dgdavid force-pushed the storage-settings-break branch from 219ae48 to 0e7051d Compare February 21, 2024 08:54

[web] Move device selection to its own section

e431192

And take the opportunity to avoid triggering the onChange callback when the user actually does not change the selected device but clicks the "Accept" button of the selection dialog.

dgdavid force-pushed the storage-settings-break branch from 0e7051d to 2491bf4 Compare February 21, 2024 12:12

[web] Create a section for file systems

f39c9af

And mount there the storage/ProposalVolumes component instead of doing so in the settings section. Note that, most probably, the code of ProposalFileSystemsSection and ProposalVolumes could be merged in a single component.

dgdavid force-pushed the storage-settings-break branch from 2491bf4 to f39c9af Compare February 21, 2024 12:18

[web] Allow sections to have a description

e708f47

Intended for adding, when needed, a short explanation about its usefulness.

dgdavid mentioned this pull request Feb 22, 2024

[web] Stop exporting all components #1049

Open

dgdavid commented Feb 22, 2024

View reviewed changes

Merge branch 'master' into storage-settings-break

b755cbe

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

dgdavid added 4 commits February 22, 2024 13:10

[web] Add descriptions to some storage sections

8c61f03

Based on @ancorgs's comment at #1045 (comment)

[web] Drop redundant text in the file systems section

1d34f4c

We've agreed that section title states almost the same.

[web] Fine tuning styles

e8896a5

[web] Render section title conditionally

93c4d21

To avoid having empty <p> HTML nodes when description is not given.

dgdavid changed the title ~~[WIP] [web] Storage page: break settings into multiple sections~~ [web] Storage page: break settings into multiple sections Feb 22, 2024

dgdavid marked this pull request as ready for review February 22, 2024 13:13

dgdavid added 3 commits February 22, 2024 14:01

[web] Move LVM setting to storage device section

ba0114d

Since, using @ancorgs's words, it is more > consistent with the future we want to go (in which LVM is part of > the device selection)

[web] Improve accessible texts used for skeletons

0c3419b

According to #1045 (comment)

[web] Update changelog file

eb8814a

dgdavid requested review from joseivanlopez and ancorgs February 22, 2024 14:11

joseivanlopez approved these changes Feb 22, 2024

View reviewed changes

web/src/components/storage/ProposalDeviceSection.jsx Outdated Show resolved Hide resolved

web/src/components/storage/ProposalSpacePolicySection.jsx Outdated Show resolved Hide resolved

web/src/components/storage/ProposalSpacePolicySection.jsx Outdated Show resolved Hide resolved

[web] Minor adjustments from code review

75bd8aa

joseivanlopez approved these changes Feb 22, 2024

View reviewed changes

dgdavid merged commit 7a6e88a into master Feb 22, 2024
2 checks passed

dgdavid deleted the storage-settings-break branch February 22, 2024 16:07

dgdavid mentioned this pull request Feb 26, 2024

initial global btrfs option #1039

Merged

imobachgs mentioned this pull request May 17, 2024

Update to release 8 #1232

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[web] Storage page: break settings into multiple sections #1045

[web] Storage page: break settings into multiple sections #1045

dgdavid commented Feb 21, 2024 •

edited

Loading

coveralls commented Feb 21, 2024 •

edited

Loading

dgdavid Feb 22, 2024

dgdavid Feb 22, 2024

dgdavid Feb 22, 2024

This comment was marked as resolved.

This comment was marked as outdated.

[web] Storage page: break settings into multiple sections #1045

[web] Storage page: break settings into multiple sections #1045

Conversation

dgdavid commented Feb 21, 2024 • edited Loading

coveralls commented Feb 21, 2024 • edited Loading

dgdavid Feb 22, 2024

Choose a reason for hiding this comment

dgdavid Feb 22, 2024

Choose a reason for hiding this comment

dgdavid Feb 22, 2024

Choose a reason for hiding this comment

This comment was marked as resolved.

This comment was marked as outdated.

dgdavid commented Feb 21, 2024 •

edited

Loading

coveralls commented Feb 21, 2024 •

edited

Loading