-
Notifications
You must be signed in to change notification settings - Fork 367
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
upcoming: [M3-7609] - Placement Groups Landing Page #10068
Conversation
bottom: -2, | ||
left: 52, | ||
position: 'absolute', | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to position the beta chip similarly to AGLB menu item since this one also wraps in two lines a bit awkwardly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I had noticed this before our huddle yesterday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all pretty boilerplate and subject to iterations as we build this feature
Coverage Report: ✅ |
packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx
Show resolved
Hide resolved
packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsLanding.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General structure looks great! Have just a few small items
...ages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.test.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/PlacementGroups/PlacementGroupsLanding/PlacementGroupsRow.tsx
Outdated
Show resolved
Hide resolved
<Link | ||
data-testid={`link-to-placement-group-${id}`} | ||
to={`/placement-groups/${id}`} | ||
> | ||
{label} ({affinityType}) | ||
</Link> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we push UX to allow us to put affinity_type
in its own column? We have plenty of room and this would allow users to order by affinity_type
(and maybe filter/seach when we improve our tables)
This also deviates from our normal pattern of keeping the label basic (with no extra information)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a good suggestion, but prob not for M1. Couple reasons:
- The PG label with the affinity appendage will follow you around (it will also be in the PG select, breadcrumb etc). It adds consistency.
- (for now) the sorting is a bit superfluous considering you only get a max of 5 PGs per account
It might be something we do in the future however as the feature evolves
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the same question, thanks for asking and clarifying. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
Description 📝
This PR adds core elements for the Placement Group feature, namely basic navigation and a populated landing page
Changes 🔄
Preview 📷
How to test 🧪
Prerequisites
Verification steps
Placement Groups
flag and MSWAs an Author I have considered 🤔
Check all that apply