upcoming: [M3-7937] - Update Placement Groups detail and summaries #10325
Conversation
abailly-akamai
force-pushed
the
M3-7937
branch
5 times, most recently
from
388c191
to
03adb1e
Compare
abailly-akamai
requested review from
hana-linode and
cpathipa
and removed request for
a team
abailly-akamai
changed the title
|
|
||
| export type DescriptionListProps = | ||
| | DescriptionListBaseProps | ||
| | DescriptionListGridProps; |
There was a problem hiding this comment.
Using TypeScript's discriminated unions here to make sure gridProps is required when "grid" displayMode is selected
| </StyledDL> | ||
| </Box> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
this is a pretty simple component that is only being used in placement groups right now, but could be used in other areas. It uses the dl > dt, dd for the markup, which - while not being semantically handled perfectly by all browsers - is still better than using paragraphs or divs like we've been doing in other places. Up to team members to implement it or not in their features.
| @@ -15,15 +15,15 @@ import { TextField, TextFieldProps } from '../TextField'; | |||
| const useStyles = makeStyles<void, 'editIcon' | 'icon'>()( | |||
| (theme: Theme, _params, classes) => ({ | |||
| button: { | |||
| '&:first-of-type': { | |||
| '&[aria-label="Save"]': { | |||
There was a problem hiding this comment.
tiny fix to tighten the edit breadcrumb button which had been given an extra let margin in a regression
|
Coverage Report: ✅ |
|
Looks good on first-appearance review. I like the addition of the |
There was a problem hiding this comment.
Functionality and code LGTM!
- Confirming create and edit drawers
- Confirming detail page with summary
- Confirming assign drawer
- Confirming
DescriptionListcomponent.
UX - Observations:
1.Redundant configuration data in multiple spots.
- Can we change the column name to
Statusinstead ofLinode statusto maintain consistency with the Linode's table and VPC subnets table?
|
Thanks @cpathipa, good observations I will take the redundant info to UX, meanwhile I did update the status column header 👍 |
Description 📝
This PR achieves two main goals:
Affinity Type)" appendagesChanges 🔄
DescriptionListcomponent for summaries (only used for PG, meant to be expanded)DescriptionListin PG drawers and PG detailAffinity Type)" appendages inPreview 📷
How to test 🧪
Prerequisites
pg-user-1(1Password)Verification steps
yarn storybookDescriptionListbehavior and styling via using the playgroundAs an Author I have considered 🤔
Check all that apply