-
Notifications
You must be signed in to change notification settings - Fork 357
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-8369] β Add "Encrypt Volume" checkbox in Edit Volume drawer #10787
upcoming: [M3-8369] β Add "Encrypt Volume" checkbox in Edit Volume drawer #10787
Conversation
Coverage Report: β
|
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.
Looks great!
packages/manager/src/features/Volumes/EditVolumeDrawer.test.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Volumes/EditVolumeDrawer.test.tsx
Outdated
Show resolved
Hide resolved
packages/manager/src/features/Volumes/EditVolumeDrawer.test.tsx
Outdated
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.
Test coverage and the different states themselves look good!
I'm curious why we decided to go with a disabled checkbox here, rather than the lock icon we've been using elsewhere to denote encrypted, especially since this setting can't be changed.
This Branch | Potentially |
---|---|
Though one difference here from elsewhere is displaying the tooltip regardless of encryption status.
I raised this with UX -- the disabled checkbox is more consistent with similar cases for LDE, and there's a chance that encrypting existing volumes will be supported down the line. |
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.
there's a chance that encrypting existing volumes will be supported down the line.
Got it. Thanks for the context!
Description π
Add a disabled "Encrypt Volume" checkbox in Edit Volume drawer that reflects the volume's encrypted status (i.e., encrypted volumes should have a checked disabled box, and unencrypted volumes should have an unchecked disabled box)
Target release date ποΈ
9/3/24
Preview π·
Encrypted volume
Unencrypted volume
How to test π§ͺ
Prerequisites
Point at the dev environment with the
blockstorage-encryption
tag on your accountVerification steps
Confirm the Edit Volume drawer has the BSE checkbox when the feature flag is on, and that the checkbox reflects the volume's encrypted status (i.e., encrypted volumes should have a checked disabled box, and unencrypted volumes should have an unchecked disabled box)
As an Author I have considered π€