Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

DEVPROD-1973: Update public keys table #2174

Merged
merged 5 commits into from
Nov 30, 2023

Conversation

sophstad
Copy link
Contributor

@sophstad sophstad commented Nov 28, 2023

DEVPROD-1973

Description

  • Convert public keys table to use LG table
  • Simplify cache mutations that can just refetch MyPublicKeys query instead

Screenshots

image

Testing

Evergreen PR

Copy link

cypress bot commented Nov 28, 2023

1 flaky test on run #14482 ↗︎

0 543 10 0 Flakiness 1
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Memoize columns
Project: Spruce Commit: 83e14b5697
Status: Passed Duration: 24:15 💡
Started: Nov 29, 2023 10:17 PM Ended: Nov 29, 2023 10:41 PM

Review all test suite changes for PR #2174 ↗︎

@sophstad sophstad requested a review from a team November 28, 2023 22:15
@sophstad sophstad marked this pull request as ready for review November 28, 2023 22:15
data: { myPublicKeys: [...data.updatePublicKey] },
});
},
refetchQueries: ["MyPublicKeys"],
Copy link
Contributor

Choose a reason for hiding this comment

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

:D

Comment on lines 72 to 82
<Button
size="small"
data-cy="edit-btn"
leftGlyph={<Icon glyph="Edit" />}
onClick={() => {
setEditModalProps({
initialPublicKey: { key, name },
visible: true,
});
}}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly could look nice if we turned these into IconButtons? its just a suggestion :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alas it looked a bit weird :( we use this action button pattern elsewhere too so I think the consistency is good!

cell: ({ row }) => {
const { key, name } = row.original;
return (
<ButtonContainer className="w-[100px]">
Copy link
Contributor

Choose a reason for hiding this comment

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

just double checking that you mean to keep this className here


return (
<BaseTable
data-cy="hosts-table"
Copy link
Contributor

Choose a reason for hiding this comment

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

data-cy should probably be updated!

@@ -131,25 +25,16 @@ export const PublicKeysTab: React.FC = () => {
});
}}
>
Add New Key
Add key
</PlusButton>
<TableContainer>
Copy link
Contributor

Choose a reason for hiding this comment

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

you could possibly remove the TableContainer here, the space between the button and table seems like a lot

@sophstad
Copy link
Contributor Author

Merging since the flake failure is also present on main

@sophstad sophstad merged commit 4fee008 into evergreen-ci:main Nov 30, 2023
2 checks passed
@sophstad sophstad deleted the DEVPROD-1973 branch November 30, 2023 14:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants