upcoming: [M3-9095] - Add Linode Interfaces Table to Linode Details #11655
Conversation
bnussman-akamai
requested review from
jdamore-linode,
harsh-akamai and
hasyed-akamai
and removed request for
a team
|
Coverage Report: ✅ |
Cloud Manager UI test results🎉 508 passing tests on test run #8 ↗︎
|
| <Table> | ||
| <TableHead> | ||
| <TableRow> | ||
| <TableCell>ID</TableCell> | ||
| <TableCell>Type</TableCell> | ||
| <TableCell>MAC Address</TableCell> | ||
| <TableCell>Version</TableCell> | ||
| <TableCell>Updated</TableCell> | ||
| <TableCell>Created</TableCell> | ||
| <TableCell /> | ||
| </TableRow> | ||
| </TableHead> |
There was a problem hiding this comment.
nit: in terms of naming, it seems like this should go in LinodeInterfacesTable too since it's part of the table
| return 'public'; | ||
| }; | ||
|
|
||
| export type InterfaceType = ReturnType<typeof getLinodeInterfaceType>; |
There was a problem hiding this comment.
could we update this name to LinodeInterfaceType? that way if someone else sees it they don't think 'configuration profile interface vs linodeInterface'
| export type InterfaceType = ReturnType<typeof getLinodeInterfaceType>; | |
| export type LinodeInterfaceType = ReturnType<typeof getLinodeInterfaceType>; |
| const { linodeId } = useParams<{ linodeId: string }>(); | ||
| const _linodeId = Number(linodeId); | ||
|
|
||
| return ( | ||
| <Stack spacing={2}> | ||
| <LinodeNetworkingSummaryPanel linodeId={_linodeId} /> | ||
| <LinodeFirewalls linodeID={_linodeId} /> | ||
| {!isLinodeInterfaceEnabled && <LinodeFirewalls linodeID={_linodeId} />} |
There was a problem hiding this comment.
are we planning to hide LinodeFirewalls for LinodeInterfaces / should we check with UX? I can assign [M3-9317] to you if you're planning to tackle it here
There was a problem hiding this comment.
Yeah, you can assign it to me! I assume we will be hiding this table because Interfaces will exist at the Interface level.
I checked the mockups again, and actually, it looks like I need to add a firewall column to the interfaces table
| @@ -18,6 +18,7 @@ export default defineConfig({ | |||
| }, | |||
| }, | |||
| server: { | |||
| allowedHosts: ['cloud.lindev.local'], | |||
|
oh I forgot to comment - I was able to see the network interfaces table! ✅ will return for a final review after firewall column is added |
Description 📝
Preview 📷
How to test 🧪
Prerequisites
Verification steps
Note
The UX mockup only included a type and MAC Addresss column.... I added more columns because I figured it can't hurt. We can refine as the project evolves but I don't see any reason to hide things from the user.
I'm surprised we're not showing the Interfaces IP addresses or attached firewall in the table row. I have a feeling we will want this... I'll ask UX next time we sync, but this isn't a blocker. We'll come back and refine as needed
Author Checklists
As an Author, to speed up the review process, I considered 🤔
👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support
As an Author, before moving this PR from Draft to Open, I confirmed ✅