feat: [WD-18263] CMS Storage pool usage #1124

Kxiru · 2025-02-25T01:24:16Z

Done

Displaying multiple pools depending on Cluster

Fixes [list issues/bugs if needed]

QA

Run the LXD-UI:
- On the demo server via the link posted by @webteam-app below. This is only available for PRs created by collaborators of the repo. Ask @mas-who or @edlerd for access.
- With a local copy of this branch, build and run as described in the docs.
Perform the following QA steps:
- Review the Storage Pool list page on a clustered and non-clustered environment.

Screenshots

webteam-app · 2025-02-25T01:24:20Z

Demo

Jenkins

demos.haus

edlerd

I think we can improve the approach for the data loading. See comment below.

src/pages/storage/StoragePoolSize.tsx

src/sass/_storage.scss

src/pages/storage/StoragePools.tsx

edlerd

This looks already very neat. Data loading code LGTM, QA of the data part as well.

The layout needs a bit of refinement, I think there are some improvements we should aim for.

edlerd · 2025-02-26T20:09:50Z

src/components/ResourceLink.tsx

@@ -11,7 +12,9 @@ interface Props {
 const ResourceLink: FC<Props> = ({ type, value, to }) => {
  return (
    <Link
-      className="p-chip is-inline is-dense resource-link"
+      className={classNames("p-chip is-inline is-dense resource-link", {
+        "storage-pool-table": true,


This shouldn't be in the generic component

This should be reverted.

src/pages/storage/StoragePoolSize.tsx

src/sass/_storage.scss

edlerd · 2025-02-26T20:28:26Z

src/components/Meter.tsx

@@ -15,7 +15,7 @@ const Meter: FC<Props> = ({
  hoverText,
 }: Props) => {
  return (
-    <>
+    <div className="p-meter-container">


Ideally we avoid those wrapper classes and find a way to style without having to add markup just for layouting.

Can we undo the changes here and in the meter.scss file? I assume with the two column solution most of it is not needed.

src/pages/storage/StoragePoolSize.tsx

Signed-off-by: Nkeiruka <[email protected]>

edlerd

The table is great. Some ideas for refinement below.

edlerd · 2025-02-27T16:18:54Z

src/pages/storage/StoragePoolSize.tsx

+            {/* {isClustered && (
+              <ResourceLink
+                to="/ui/cluster"
+                type={
+                  hasMemberSpecificSize ? "cluster-member" : "cluster-group"
+                }
+                value={
+                  hasMemberSpecificSize && poolResource.memberName
+                    ? poolResource.memberName
+                    : "Cluster wide"
+                }
+              />
+            )} */}


Suggested change

{/* {isClustered && (

<ResourceLink

to="/ui/cluster"

type={

hasMemberSpecificSize ? "cluster-member" : "cluster-group"

}

value={

hasMemberSpecificSize && poolResource.memberName

? poolResource.memberName

: "Cluster wide"

}

/>

)} */}

edlerd · 2025-02-27T16:20:36Z

src/pages/storage/StoragePoolSize.tsx

+        }
+
+        return (
+          <div className="storage-pool-meter-container" key={index}>


Suggested change

<div className="storage-pool-meter-container" key={index}>

<Fragment key={index}>

I think this is not needed.

edlerd · 2025-02-27T16:20:50Z

src/sass/_storage.scss

+.storage-pool-meter-container {
+  align-items: center;
+  display: flex;
+}


Suggested change

.storage-pool-meter-container {

align-items: center;

display: flex;

}

Seems not needed.

edlerd · 2025-02-27T16:23:08Z

src/sass/_storage.scss

+.storage-pool-overview-meter {
+  display: flex;
+}


Suggested change

.storage-pool-overview-meter {

display: flex;

}

.cluster-member-with-meters {

display: flex;

}

Better name and move this into the namespace for storage-overview-tab. This doesn't need to be on the root namespace.

edlerd · 2025-02-27T16:24:38Z

src/pages/storage/StoragePoolOverview.tsx

@@ -38,7 +45,10 @@ const StoragePoolOverview: FC<Props> = ({ pool, project }) => {
              </tr>
              <tr>
                <th className="u-text--muted">Size</th>
-                <td>
+                <td className="storage-pool-overview-meter">


Suggested change

<td className="storage-pool-overview-meter">

<td className="cluster-member-with-meters">

The styling of this one needs adjustments. The height of the member names should be aligned with the meters and there needs to be some gap between the chips and the meters.

edlerd · 2025-02-27T16:26:38Z

src/pages/storage/StoragePoolSelectTable.tsx

+          ? [
+              {
+                content: <StoragePoolClusterMember pool={pool} />,
+                role: "rowheader",


Suggested change

role: "rowheader",

role: "cell",

edlerd · 2025-02-27T16:28:13Z

src/components/ResourceLink.tsx

@@ -11,7 +12,9 @@ interface Props {
 const ResourceLink: FC<Props> = ({ type, value, to }) => {
  return (
    <Link
-      className="p-chip is-inline is-dense resource-link"
+      className={classNames("p-chip is-inline is-dense resource-link", {
+        "storage-pool-table": true,


This should be reverted.

edlerd · 2025-02-27T16:30:01Z

src/components/Meter.tsx

@@ -15,7 +15,7 @@ const Meter: FC<Props> = ({
  hoverText,
 }: Props) => {
  return (
-    <>
+    <div className="p-meter-container">


Can we undo the changes here and in the meter.scss file? I assume with the two column solution most of it is not needed.

edlerd · 2025-02-27T16:35:28Z

src/sass/_resources.scss

+.clustered-resource-link {
+  height: 2.25rem;
+}


This is only for the storage-pool-table and the migrate-instance-table > storage-pools. On the storage pool overview page, the height should be adjusted to match its context.

I suggest moving this into the SCSS files for the respective use cases instead of the global size in here.

edlerd · 2025-02-27T16:38:59Z

src/pages/storage/StoragePools.tsx

Maybe hide the usage and cluster member columns on smaller screen sizes?

Because on small screen sizes, the columns and headings become unreadable. We can hide the columns with css and a media query below a reasonable breakpoint.

Kxiru requested a review from edlerd February 25, 2025 01:24

Kxiru self-assigned this Feb 25, 2025

Kxiru force-pushed the cms-pool-usage branch 2 times, most recently from b27c6b5 to 7d0c01c Compare February 25, 2025 02:03

edlerd reviewed Feb 25, 2025

View reviewed changes

src/pages/storage/StoragePoolSize.tsx Outdated Show resolved Hide resolved

Kxiru force-pushed the cms-pool-usage branch from 7d0c01c to 82e8949 Compare February 25, 2025 16:38

Kxiru requested a review from edlerd February 25, 2025 16:39

Kxiru force-pushed the cms-pool-usage branch from 82e8949 to ca621f0 Compare February 25, 2025 16:57

edlerd reviewed Feb 26, 2025

View reviewed changes

Kxiru force-pushed the cms-pool-usage branch from ca621f0 to 59a64d9 Compare February 26, 2025 19:23

Kxiru requested a review from edlerd February 26, 2025 19:23

edlerd reviewed Feb 26, 2025

View reviewed changes

src/pages/storage/StoragePoolSize.tsx Show resolved Hide resolved

Kxiru force-pushed the cms-pool-usage branch 2 times, most recently from 508e565 to 47f2066 Compare February 27, 2025 12:16

Kxiru marked this pull request as ready for review February 27, 2025 12:24

Kxiru requested a review from edlerd February 27, 2025 12:24

feat: [WD-18263] CMS Storage pool usage

02b3c3c

Signed-off-by: Nkeiruka <[email protected]>

Kxiru force-pushed the cms-pool-usage branch from 47f2066 to 02b3c3c Compare February 27, 2025 12:26

edlerd reviewed Feb 27, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: [WD-18263] CMS Storage pool usage #1124

feat: [WD-18263] CMS Storage pool usage #1124

Kxiru commented Feb 25, 2025 •

edited

Loading

webteam-app commented Feb 25, 2025

edlerd left a comment

edlerd left a comment

edlerd Feb 26, 2025

edlerd Feb 27, 2025

edlerd Feb 26, 2025

edlerd Feb 27, 2025

edlerd left a comment

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

edlerd Feb 27, 2025

	<div className="storage-pool-meter-container" key={index}>
	<Fragment key={index}>

	<td className="storage-pool-overview-meter">
	<td className="cluster-member-with-meters">

feat: [WD-18263] CMS Storage pool usage #1124

Are you sure you want to change the base?

feat: [WD-18263] CMS Storage pool usage #1124

Conversation

Kxiru commented Feb 25, 2025 • edited Loading

Done

QA

Screenshots

webteam-app commented Feb 25, 2025

edlerd left a comment

Choose a reason for hiding this comment

edlerd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edlerd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kxiru commented Feb 25, 2025 •

edited

Loading