Skip to content

Link clusterName in image automation details#4035

Merged
TheGostKasper merged 3 commits intomainfrom
link-cluster-in-imageautomation-details
Sep 21, 2023
Merged

Link clusterName in image automation details#4035
TheGostKasper merged 3 commits intomainfrom
link-cluster-in-imageautomation-details

Conversation

@TheGostKasper
Copy link
Copy Markdown
Contributor

part of ee 3339

What changed?

  • Link clusterName in ImageAutomation detail page
  • Filter HeaderRows visible items before looping over its items

@TheGostKasper TheGostKasper added the area/ui Issues that require front-end work label Sep 20, 2023
return (
h.visible !== false && (
{items
.filter((h) => h.visible !== false)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can replace this line with .filter((h) => h.visible)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And there is another line like that in HeaderRows.test.tsx:

if (h.visible !== false) {

which can be replaced with

if (h.visible) {

Optional. but since you are working on these files anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not all the items contains visible:true so that we deal with it as a default value for items to be visible:true

for example:

{
    rowkey: "Policy Name",
    value: "Controller ServiceAccount Tokens Automount",
  },
  {
    rowkey: "Application",
    value: "flux-system/external-secrets",
  },
  {
    rowkey: "Violation Time",
    value: "15 minutes ago",
  },
  {
    rowkey: "Category",
    value: " weave.categories.access-control",
    visible: false,
  },

here we want only to filter over what's not false to save extra checks & make visible required

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I see, thanks for the explanation.

Copy link
Copy Markdown
Contributor

@opudrovs opudrovs left a comment

Choose a reason for hiding this comment

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

LGTM. 🎉

@TheGostKasper TheGostKasper merged commit e9b2491 into main Sep 21, 2023
@TheGostKasper TheGostKasper deleted the link-cluster-in-imageautomation-details branch September 21, 2023 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ui Issues that require front-end work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants