feat: [M3-7351] - AGLB Configurations Add Route Drawer and other refinements #9853
Conversation
bnussman-akamai
added
Work in Progress
ACLB
bnussman-akamai
requested review from
jdamore-linode and
cliu-akamai
and removed request for
a team
| @@ -204,50 +187,6 @@ export const RoutesTable = ({ configuredRoutes }: Props) => { | |||
|
|
|||
| return ( | |||
| <> | |||
| <Stack | |||
There was a problem hiding this comment.
Moved this logic outside of this component so we can re-use it between the Routes tab and the configurations tab
| <ActionMenu | ||
| actionsList={[ | ||
| { onClick: () => onEditRoute(route), title: 'Edit' }, | ||
| { onClick: () => null, title: 'Clone Route' }, |
There was a problem hiding this comment.
We're not going to do cloning for beta
There was a problem hiding this comment.
I tested Add Route drawer functionality and confirmed that adding a new route sends a POST request and form validation works for both adding a new route and adding an existing route.
I haven't seen any major styling issues, but a have commented a couple suggestions related to service target table and the config tab. I'll take another glance through the UI to finish confirming styling after this initial feedback.
I also don't know the best spot for this feedback and know copy is being finalized anyway, but I don't want to accidentally miss these if this copy doesn't change much - Configure tab > Apply Certificate drawer:
packages/manager/.changeset/pr-9853-upcoming-features-1698705643963.md
Outdated
Show resolved
Hide resolved
...ager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationAccordion.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerRoutes.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/LoadBalancers/LoadBalancerDetail/LoadBalancerServiceTargets.tsx
Show resolved
Hide resolved
...ager/src/features/LoadBalancers/LoadBalancerDetail/Configurations/ConfigurationAccordion.tsx
Show resolved
Hide resolved
|
Updated copy will be coming very soon! @mjac0bs I think I addressed your feedback! Let me know if I missed anything! |
packages/manager/.changeset/pr-9853-upcoming-features-1698871857708.md
Outdated
Show resolved
Hide resolved
| { country: 'jp', id: 'jp-osa', label: 'Osaka, JP' }, | ||
| { country: 'au', id: 'ap-southeast', label: 'Sydney, AU' }, | ||
| ]; | ||
| import { LoadBalancerRegions as Regions } from '../LoadBalancerDetail/LoadBalancerRegions'; |
There was a problem hiding this comment.
Can we keep LoadBalancerRegions as is and rename component name to LoadBalancerRegionsPanel
| @@ -29,11 +29,30 @@ interface Props { | |||
| loadbalancerId: number; | |||
| } | |||
|
|
|||
| function getConfigurationPayloadFromConfiguration( | |||
There was a problem hiding this comment.
Can we name it buildConfigurationPayload / generateConfigPayload ? getConfigurationPayloadFromConfiguration is a bit confusing.
There was a problem hiding this comment.
Add Routes drawer still looks good, error state for Configurations tab looks good, and thanks for adding the ID column to Service Targets (and ID elsewhere, it looks like).
Non-blocking: one more spot in the mocks where we have an ID -- the certificate ID and created timestamp -- can we add that here now?:
Worth asking because there are are little changes in this PR -- are there any additional unit or e2e updates we want to make to check for the IDs displaying in the UI, etc?
Description 📝
RouteSelectcomponentPreview 📷
How to test 🧪
Prerequisites
Verification steps
Add Routebutton and the corresponding drawer (We can only test so much with the MSW)As an Author I have considered 🤔