Skip to content

Update copy regarding disk attach / detach requirements #2207

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.

Sign up for GitHub

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

Merged
charliepark merged 3 commits into from
Apr 27, 2024
Merged

Update copy regarding disk attach / detach requirements #2207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3547f1f
Update copy on disk attach / detach states
charliepark Apr 26, 2024
34f6d3f
Update tests
charliepark Apr 26, 2024
dfea838
slight copy tweak
charliepark Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions app/pages/project/instances/instance/tabs/StorageTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,6 @@ const staticCols = [
colHelper.accessor('timeCreated', Columns.timeCreated),
]

const attachableStates = fancifyStates(instanceCan.attachDisk.states)
const detachableStates = fancifyStates(instanceCan.detachDisk.states)

export function StorageTab() {
const [showDiskCreate, setShowDiskCreate] = useState(false)
const [showDiskAttach, setShowDiskAttach] = useState(false)
Expand Down Expand Up @@ -128,7 +125,10 @@ export function StorageTab() {
{
label: 'Detach',
disabled: !instanceCan.detachDisk(instance) && (
<>Instance must be in state {detachableStates} before disk can be detached</>
<>
Instance must be <span className="text-default">stopped</span> before disk can
be detached
Copy link
Collaborator

@david-crespo david-crespo Apr 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

might as well do what the other ones do, fudge it grammatically to save a few words: “instance must be stopped to detach disks” or maybe even “instance must be stopped” by itself? the button is disabled and it says “detach”

</>
),
onActivate() {
detachDisk.mutate({ body: { disk: disk.name }, ...instancePathQuery })
Expand Down Expand Up @@ -174,7 +174,12 @@ export function StorageTab() {
<Button
size="sm"
onClick={() => setShowDiskCreate(true)}
disabledReason={<>Instance must be {attachableStates} to create a disk</>}
disabledReason={
<>
Instance must be <span className="text-default">stopped</span> to create and
attach a disk
</>
}
Copy link
Collaborator

@david-crespo david-crespo Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could it be worth saying “create and attach” here? it’s the attach that’s the problem, right?

disabled={!instanceCan.attachDisk(instance)}
>
Create new disk
Expand All @@ -183,15 +188,21 @@ export function StorageTab() {
variant="secondary"
size="sm"
onClick={() => setShowDiskAttach(true)}
disabledReason={<>Instance must be {attachableStates} to attach a disk</>}
disabledReason={
<>
Instance must be <span className="text-default">stopped</span> to attach a
disk
</>
}
disabled={!instanceCan.attachDisk(instance)}
>
Attach existing disk
</Button>
</div>
{!instanceCan.attachDisk(instance) && (
<span className="max-w-xs text-sans-md text-tertiary">
A disk cannot be added or attached unless the instance is {attachableStates}.
The instance must be <span className="text-default">stopped</span> to add or
attach a disk.
</span>
)}
</div>
Expand Down
5 changes: 2 additions & 3 deletions test/e2e/instance-disks.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,7 @@ import {
test('Attach disk', async ({ page }) => {
await page.goto('/projects/mock-project/instances/db1')

const warning =
'A disk cannot be added or attached unless the instance is creating or stopped.'
const warning = 'The instance must be stopped to add or attach a disk.'
await expect(page.getByText(warning)).toBeVisible()

const row = page.getByRole('row', { name: 'disk-1', exact: false })
Expand All @@ -30,7 +29,7 @@ test('Attach disk', async ({ page }) => {
await expect(page.getByRole('menuitem', { name: 'Detach' })).toBeDisabled()
await page.getByRole('menuitem', { name: 'Detach' }).hover()
await expect(
page.getByText('Instance must be in state creating, stopped, or failed')
page.getByText('Instance must be stopped before disk can be detached')
).toBeVisible()
await page.keyboard.press('Escape') // close menu

Expand Down