upcoming: [UIE-9512, UIE-9530] - New Rule Set Details Drawer with Marked for Deletion status#13108
Conversation
…l-ruleset-row-layout
…l-ruleset-row-layout
…l-ruleset-row-layout
…l-ruleset-row-layout
…l-ruleset-row-layout
…layout' into UIE-9514-update-firewall-rule-drawer-to-support-referencing-ruleset
…with-marked-for-deletion
packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleSetDetailsView.tsx
Show resolved
Hide resolved
packages/manager/src/features/Firewalls/FirewallDetail/Rules/FirewallRuleTable.tsx
Show resolved
Hide resolved
| <ActionsPanel | ||
| primaryButtonProps={{ | ||
| label: 'Cancel', | ||
| onClick: closeDrawer, | ||
| }} | ||
| /> |
There was a problem hiding this comment.
UX observation: It seems very untraditional to use a Primary button for a Cancel/Close button.
Everywhere else in Cloud Manager will use a secondary button if the action is Close/Cancel
There was a problem hiding this comment.
Yes, we can do that for consistency. I'll check and confirm with UX cc @tzmiivsk-akamai
There was a problem hiding this comment.
Agreed, let's keep Cancel button as secondary for consistency. I'll update the mockups
| <StyledChip | ||
| action={rule.action} | ||
| label={capitalize(rule.action?.toLowerCase() ?? '')} | ||
| /> |
There was a problem hiding this comment.
The UX mockups may not agree, but I think it would be much better to just use our Chip "out-of-the-box" rather than making a custom styled component.
I don't think a custom styled Chip is worth the maintenance burden and the possible deviation from other Chips throughout Cloud Manager.
| <StyledChip | |
| action={rule.action} | |
| label={capitalize(rule.action?.toLowerCase() ?? '')} | |
| /> | |
| <Chip | |
| color={rule.action === 'ACCEPT' ? 'success' : 'error'} | |
| label={capitalize(rule.action?.toLowerCase() ?? '')} | |
| /> |
| This PR | Suggestion (Use "out of the box" Chip) | |
|---|---|---|
| Light Mode |  |
 |
| Dark Mode |  |
 |
There was a problem hiding this comment.
I'll check with UX regarding this as well. But I'm merging this as is since it blocking other PRs
There was a problem hiding this comment.
The thing is that the current “out-of-the-box” badge styles are expected to be aligned with the ADS styles soon, meaning they will eventually migrate to ADS. In the mockups, I’m already using the ADS badge component styles.
If needed - and if it significantly simplifies the implementation - I’m okay with reusing the current CM component and its colors. But if it doesn’t affect development much, then let’s proceed with the ADS badges.
Cloud Manager UI test results🔺 1 failing test on test run #26 ↗︎
Details
TroubleshootingUse this command to re-run the failing tests: pnpm cy:run -s "cypress/e2e/core/firewalls/firewall-landing-page.spec.ts" |
|||||||||||||||||
|
Merging - verified that the test failure is unrelated to this PR |
pmakode-akamai
added
Approved
6 participants
Description 📝
Changes 🔄
⚠️ Marked for deletionstatus.Scope 🚢
Target release date 🗓️
N/A
Preview 📷
firewall-ruleset-1)ruleset-with-a-longer-32ch-label)How to test 🧪
Prerequisites
firewallRulesetsPrefixlistsfeature flagVerification steps
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 ✅