Skip to content

web: Better storage summary#2003

Merged
ancorgs merged 1 commit intoagama-project:storage-config-uifrom
ancorgs:storage-ui-summary
Feb 17, 2025
Merged

web: Better storage summary#2003
ancorgs merged 1 commit intoagama-project:storage-config-uifrom
ancorgs:storage-ui-summary

Conversation

@ancorgs
Copy link
Contributor

@ancorgs ancorgs commented Feb 17, 2025

Problem

When the configuration uses some feature not supported by the UI (like LVM or legacyAutoyastStorage), the storage section of the summary shows this message: "No device selected yet". Even if the configuration leaded to a valid storage setup.

Solution

Now the situation is properly handled and the message makes more sense.

{drives.length === 1 && <SingleDiskSummary drive={drives[0]} />}
{drives.length > 1 && <MultipleDisksSummary drives={drives} />}
<Content>
{configModel && <ModelSummary model={configModel} />}
Copy link
Contributor

@joseivanlopez joseivanlopez Feb 17, 2025

Choose a reason for hiding this comment

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

NP: do we need to pass the model as prop? Just saying because after some discussions with @dgdavid, we decided that the components should recover all the info they need from hooks. But maybe in this case we want a component which is able to render different models?

Copy link
Contributor

Choose a reason for hiding this comment

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

Exactly. Somehow we realize that it's better having components able to query the information they need when possible instead of depending on the props of some parent. Not written in stone, of course, but thanks to the React Query cache most of times it works fine.

But maybe in this case we want a component which is able to render different models?

Good point.

Copy link
Contributor

@joseivanlopez joseivanlopez left a comment

Choose a reason for hiding this comment

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

What happens if there is a config with errors?

@ancorgs
Copy link
Contributor Author

ancorgs commented Feb 17, 2025

What happens if there is a config with errors?

Nothing special. The summary sentence simply explains what the configuration is about, it does not tell anything about whether is a good or bad configuration. That's the mission for the part of the interface displaying issues. That's not new in my PR, that's how the summary works.

So the only change here is to better handle the situation in which we cannot explain the configuration.

@dgdavid
Copy link
Contributor

dgdavid commented Feb 17, 2025

What happens if there is a config with errors?

Nothing special. The summary sentence simply explains what the configuration is about, it does not tell anything about whether is a good or bad configuration. That's the mission for the part of the interface displaying issues. That's not new in my PR, that's how the summary works.

So the only change here is to better handle the situation in which we cannot explain the configuration.

Exactly.

There is room for improvements, though. But it has to wait until reserving time for improving the overview page itself, where these summaries are being rendered right now.

@ancorgs ancorgs merged commit fd5f65a into agama-project:storage-config-ui Feb 17, 2025
1 check passed
@imobachgs imobachgs mentioned this pull request Feb 26, 2025
imobachgs added a commit that referenced this pull request Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments