Inconsistencies in the volumes

[ancorgs]

Before merging #1104

After merging that pull request

a) It's not longer possible to add a /home volume although it's in the TW configuration
b) The Btrfs switcher is not in sync with the table of volumes

[dgdavid]

b) The Btrfs switcher is not in sync with the table of volumes

For this one the problem is in the initialization of the internal state of the component (isChecked). Such an initialization only happens once, when the component is mounted. But not when re-rendered. To understand this I recommend reading https://react.dev/learn/preserving-and-resetting-state.

We can solve it by adding a useEffect depending on the initialChecked value, but this is somehow sub-optimal: although you're going to have the expected behavior without visual flickering, the component will get an extra re-render every time the switch changes its state.

For me, the solution is make that component stateless At the end, it is always dispatching the given onChange callback, which as a result will re-render the component because a settings prop change.

diff --git a/web/src/components/storage/ProposalSettingsSection.jsx b/web/src/components/storage/ProposalSettingsSection.jsx
index 969d862b..871024d3 100644
--- a/web/src/components/storage/ProposalSettingsSection.jsx
+++ b/web/src/components/storage/ProposalSettingsSection.jsx
@@ -131,16 +131,14 @@ const SnapshotsField = ({
 }) => {
   const rootVolume = (settings.volumes || []).find((i) => i.mountPath === "/");
 
-  const initialChecked = rootVolume !== undefined && hasFS(rootVolume, "Btrfs") && rootVolume.snapshots;
-  const [isChecked, setIsChecked] = useState(initialChecked);
-
   // no root volume is probably some error or still loading
   if (rootVolume === undefined) {
     return <Skeleton width="25%" />;
   }
 
+  const isChecked = rootVolume !== undefined && hasFS(rootVolume, "Btrfs") && rootVolume.snapshots;
+
   const switchState = (_, checked) => {
-    setIsChecked(checked);
     onChange({ active: checked, settings });
   };

You may be wondering why this happens now and not before. The answer is in a change I did at 232096b, removing a not needed internal component in src/components/storage/ProposalPage.jsx. It was being redefined each time props changed. I.e., it was a different component every time, reason why all state pieces in the chain were removed and new ones were initialized instead of reusing the (no longer) existing ones. As you can see, this internal component introduced and hidden a not optimal behavior. So, we should be careful with them.

[dgdavid]

b) The Btrfs switcher is not in sync with the table of volumes

....

removing a not needed internal component in src/components/storage/ProposalPage.jsx. It was being redefined each time props changed. I.e., it was a different component every time, reason why all state pieces in the chain were removed and new ones were initialized instead of reusing the (no longer) existing ones. As you can see, this internal component introduced and hidden a not optimal behavior. So, we should be careful with them.

Just in case the above is not clear enough, I've searched for an article explaining it better. I found this one: https://dev.to/shameel/the-terrible-react-anti-pattern-you-must-avoid-4el8. Hope it helps for a better understanding.

[ancorgs]

a) It's not longer possible to add a /home volume although it's in the TW configuration

This is caused because the following line was deleted by mistake from PageProposal.jsx during the merge

volumeTemplates={state.volumeTemplates}

[dgdavid]

a) It's not longer possible to add a /home volume although it's in the TW configuration

This is caused because the following line was deleted by mistake from PageProposal.jsx during the merge

volumeTemplates={state.volumeTemplates}

Probably was removed by me when solving a conflict in that file 😢