Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(images): Implement design for image table MAASENG-4360 #5628

Conversation

rumanowskak
Copy link
Contributor

@rumanowskak rumanowskak commented Feb 27, 2025

Done

  • Added tooltip for delete icon in Images Table
  • Aligned header text starting from "Release title" to the text "Ubuntu" in header belowe

QA steps

  • On MAAS-UI go to configuration/images
  • Hover over delete icon to see the tooltip
  • Look at header if "Release title" is aligned with the "Ubuntu" text

Fixes

Resolves MAASENG-4360

Screenshots

images_table

@webteam-app
Copy link

Copy link
Contributor

@abuyukyi101198 abuyukyi101198 left a comment

Choose a reason for hiding this comment

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

Looks good! However, there is an easy miss in the design, where when aligning the "Release title" to the left, the other column headers chould not move, as in all column headers should be left-aligned properly.

Screenshot 2025-02-27 at 17 34 38

Managing to shift the second header as the same amount as the width of the now display: none .select should align all headers correctly.

@rumanowskak
Copy link
Contributor Author

rumanowskak commented Feb 27, 2025

@abuyukyi101198 I tried to just shift the second header "Architecture" but it didn't shift the rest of them, so I did it this way.

Copy link
Contributor

@abuyukyi101198 abuyukyi101198 left a comment

Choose a reason for hiding this comment

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

The alignment seems to shift in later headers. CSS may not be enough to fix the alignment.

Comment on lines 18 to 20
th:not(.release):not(.group-select) {
padding-left: 2rem;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a generic component, we cannot directly refer to image-specific classes like .release. It seems that the alignment is not solvable with pure CSS. Maybe we can add an empty th element after the first visible header programmatically. I'm not a huge fan of this solution, but CSS just doesn't seem to work.

In GenericTable.tsx:

<DynamicTable className="p-generic-table" variant={variant}>
      <thead>
        {table.getHeaderGroups().map((headerGroup) => (
          <tr key={headerGroup.id}>
            {headerGroup.headers.filter(filterHeaders).map((header, index) => (
              <>
                <TableHeader header={header} key={header.id} />
                {index === 2 ? <th className="select-alignment" /> : null} <-- add this aligner th
              </>
            ))}
          </tr>
        ))}
      </thead>

And in GenricTable/_index.scss:

.group-select,
  .select,
  .select-alignment { <-- add select-alignment
    width: 2rem;
  }

Copy link
Contributor

@abuyukyi101198 abuyukyi101198 left a comment

Choose a reason for hiding this comment

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

LGTM!

@rumanowskak rumanowskak merged commit 7e90fbb into canonical:main Feb 28, 2025
8 checks passed
@rumanowskak rumanowskak deleted the feat-implement-image-table-design-MAASENG-4360 branch February 28, 2025 11:05
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.

3 participants