Move Owner option to Details tab

Align the podman create image design with the in progress machines dialog redesign. The owner option is now moved to the details tab and changed to a radio button so it's more clear it's a form option. The container name input can now take up the full input which is nicer on mobile devices.
cockpit-project · Mar 15, 2022 · cd9589b · cd9589b
1 parent 3027ead
commit cd9589b
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 60 deletions.
diff --git a/src/ImageRunModal.jsx b/src/ImageRunModal.jsx
@@ -7,8 +7,8 @@ import {
     FormSelect, FormSelectOption,
     Grid, GridItem,
     HelperText, HelperTextItem,
-    Modal, Select, SelectVariant,
-    SelectOption, SelectGroup, Stack,
+    Modal, Radio, Select, SelectVariant,
+    SelectOption, SelectGroup,
     TextInput, Tabs, Tab, TabTitleText,
     ToggleGroup, ToggleGroupItem,
     Flex, FlexItem,
@@ -317,7 +317,6 @@ export class ImageRunModal extends React.Component {
             memoryUnit: 'MB',
             validationFailed: {},
             volumes: [],
-            runImage: true,
             restartPolicy: "no",
             restartTries: 5,
             pullLatestImage: false,
@@ -333,7 +332,6 @@ export class ImageRunModal extends React.Component {
             searchByRegistry: 'all',
         };
         this.getCreateConfig = this.getCreateConfig.bind(this);
-        this.onCreateClicked = this.onCreateClicked.bind(this);
         this.onValueChanged = this.onValueChanged.bind(this);
     }
 
@@ -462,9 +460,9 @@ export class ImageRunModal extends React.Component {
                 });
     }
 
-    async onCreateClicked() {
+    async onCreateClicked(runImage = false) {
         const createConfig = this.getCreateConfig();
-        const { runImage, pullLatestImage } = this.state;
+        const { pullLatestImage } = this.state;
         const isSystem = this.isSystem();
         let imageExists = true;
 
@@ -664,9 +662,9 @@ export class ImageRunModal extends React.Component {
     debouncedInputChanged = debounce(300, this.handleImageSelectInput);
 
     handleOwnerSelect = (_, event) => {
-        const id = event.currentTarget.id;
+        const value = event.currentTarget.value;
         this.setState({
-            owner: id
+            owner: value
         });
     }
 
@@ -812,31 +810,30 @@ export class ImageRunModal extends React.Component {
         );
 
         const defaultBody = (
-            <Form isHorizontal={activeTabKey == 0}>
-                <Flex className="run-image-dialog-header pf-c-form pf-m-horizontal" justifyContent={{ default: 'justifyContentSpaceBetween' }}>
-                    <FormGroup fieldId='run-image-dialog-name' label={_("Name")}>
-                        <TextInput id='run-image-dialog-name'
+            <Form>
+                <FormGroup fieldId='run-image-dialog-name' label={_("Name")} className="ct-m-horizontal">
+                    <TextInput id='run-image-dialog-name'
                            className="image-name"
                            placeholder={_("Container name")}
                            value={dialogValues.containerName}
                            onChange={value => this.onValueChanged('containerName', value)} />
-                    </FormGroup>
-                    { this.props.userServiceAvailable && this.props.systemServiceAvailable &&
-                    <FormGroup fieldId='run-image-dialog-owner' label={_("Owner")}>
-                        <ToggleGroup aria-label={_("Default with single selectable")}>
-                            <ToggleGroupItem text={_("System")} buttonId="system" isSelected={owner === "system"}
-                                             onChange={this.handleOwnerSelect} />
-                            <ToggleGroupItem text={cockpit.format("$0 $1", _("User:"), this.props.user)}
-                                             buttonId={this.props.user}
-                                             isSelected={owner === this.props.user}
-                                             onChange={this.handleOwnerSelect} />
-                        </ToggleGroup>
-                    </FormGroup>
-                    }
-                </Flex>
+                </FormGroup>
                 <Tabs activeKey={activeTabKey} onSelect={this.handleTabClick}>
                     <Tab eventKey={0} title={<TabTitleText>{_("Details")}</TabTitleText>} className="pf-c-form pf-m-horizontal">
-
+                        { this.props.userServiceAvailable && this.props.systemServiceAvailable &&
+                        <FormGroup isInline hasNoPaddingTop fieldId='run-image-dialog-owner' label={_("Owner")}>
+                            <Radio value="system"
+                                   label={_("System")}
+                                   id="run-image-dialog-owner-system"
+                                   isChecked={owner === "system"}
+                                   onChange={this.handleOwnerSelect} />
+                            <Radio value={this.props.user}
+                                   label={cockpit.format("$0 $1", _("User:"), this.props.user)}
+                                   id="run-image-dialog-owner-user"
+                                   isChecked={owner === this.props.user}
+                                   onChange={this.handleOwnerSelect} />
+                        </FormGroup>
+                        }
                         <FormGroup fieldId="create-image-image-select-typeahead" label={_("Image")}
                           labelIcon={!this.props.image &&
                               <Popover aria-label={_("Image selection help")}
@@ -998,12 +995,6 @@ export class ImageRunModal extends React.Component {
                             }
                         </Grid>
                         }
-                        <FormGroup fieldId='run-image-dialog-start-after-creation' label={_("Options")} hasNoPaddingTop>
-                            <Stack hasGutter>
-                                <Checkbox isChecked={this.state.runImage} id="run-image-dialog-start-after-creation"
-                                      onChange={value => this.onValueChanged('runImage', value)} label={_("Start after creation")} />
-                            </Stack>
-                        </FormGroup>
                     </Tab>
                     <Tab eventKey={1} title={<TabTitleText>{_("Integration")}</TabTitleText>} id="create-image-dialog-tab-integration" className="pf-c-form">
 
@@ -1054,7 +1045,10 @@ export class ImageRunModal extends React.Component {
                    title={this.props.pod ? cockpit.format(_("Create container in $0"), this.props.pod.Name) : _("Create container")}
                    footer={<>
                        {this.state.dialogError && <ErrorNotification errorMessage={this.state.dialogError} errorDetail={this.state.dialogErrorDetail} />}
-                       <Button variant='primary' onClick={this.onCreateClicked} isDisabled={!image && selectedImage === ""}>
+                       <Button variant='primary' id="create-image-create-run-btn" onClick={() => this.onCreateClicked(true)} isDisabled={!image && selectedImage === ""}>
+                           {_("Create and run")}
+                       </Button>
+                       <Button variant='secondary' id="create-image-create-btn" onClick={() => this.onCreateClicked(false)} isDisabled={!image && selectedImage === ""}>
                            {_("Create")}
                        </Button>
                        <Button variant='link' className='btn-cancel' onClick={ this.props.close }>

diff --git a/src/ImageRunModal.scss b/src/ImageRunModal.scss
@@ -1,3 +1,5 @@
+@import "global-variables";
+
 .dynamic-form-group {
     .pf-c-empty-state {
         padding: 0;
@@ -54,18 +56,6 @@
     }
 }
 
-.pf-c-form.run-image-dialog-header {
-    // The pf-l-flex class is not applied because of pf-c-form.pf-m-horizontal forces grid
-    display: flex;
-    row-gap: var(--pf-global--spacer--md);
-
-    // Set max width to the image name - otherwise it often makes the owner wrap unnecessarily
-    .image-name {
-        max-width: 10rem;
-    }
-}
-
-
  // PF4 does not yet support multiple form fields for the same label
 .ct-input-group-spacer-sm.pf-l-flex {
     // Limit width for select entries and inputs in the input groups otherwise they take up the whole space
@@ -77,3 +67,13 @@
 .run-image-dialog-restart-retries {
     max-width: 5ch;
 }
+
+// HACK: A local copy of pf-m-horizontal (as ct-m-horizontal),
+// but applied at the FormGroup level instead of Form
+@media (min-width: $pf-global--breakpoint--md) {
+    .pf-c-form__group.ct-m-horizontal {
+        display: grid;
+        grid-column-gap: var(--pf-c-form--m-horizontal__group-label--md--GridColumnGap);
+        grid-template-columns: var(--pf-c-form--m-horizontal__group-label--md--GridColumnWidth) var(--pf-c-form--m-horizontal__group-control--md--GridColumnWidth);
+    }
+}
diff --git a/test/check-application b/test/check-application
@@ -1256,7 +1256,7 @@ class TestApplication(testlib.MachineCase):
         b.wait_not_present("#run-image-dialog-pull-latest-image")
 
         # Create Container, image is pulled and should end up being "running"
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         sel = "> span:not(.downloading)"
         b.wait(lambda: self.getContainerAttr(container_name, "State", sel) in 'Running')
         output = self.execute(auth, f"podman exec {container_name} ls -lh /latest || true").strip()
@@ -1285,7 +1285,7 @@ class TestApplication(testlib.MachineCase):
         b.set_checked("#run-image-dialog-pull-latest-image", True)
 
         # Create Container, image is pulled and should end up being "running"
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         sel = "> span:not(.downloading)"
         b.wait(lambda: self.getContainerAttr(container_name, "State", sel) in 'Running')
         # Verify that the latest file exists
@@ -1298,7 +1298,7 @@ class TestApplication(testlib.MachineCase):
             b.click("#containers-containers button.pf-c-button.pf-m-primary")
 
             # Start container as admin
-            b.click('button.pf-c-toggle-group__button:contains("User: admin")')
+            b.click('#run-image-dialog-owner-user')
 
             # Create Container, image is pulled and should end up being "Running"
             b.set_input_text("#run-image-dialog-name", container_name)
@@ -1307,7 +1307,7 @@ class TestApplication(testlib.MachineCase):
             b.click('button.pf-c-toggle-group__button:contains("Local")')
             b.click('button.pf-c-select__menu-item:contains("quay.io/libpod/busybox:latest")')
 
-            b.click('.pf-c-modal-box__footer button:contains("Create")')
+            b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
             b.wait(lambda: self.getContainerAttr(container_name, "State", sel) in 'Running')
 
     @testlib.nondestructive
@@ -1468,7 +1468,7 @@ class TestApplication(testlib.MachineCase):
         b.set_input_text('#run-image-dialog-volume-1-container-path', '/tmp/rw')
         rw_label = m.execute(f"ls -dZ {rwdir}").split(" ")[0]
 
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         b.wait_not_present("div.pf-c-modal-box")
         self.waitContainerRow("busybox:latest")
         sha = self.execute(auth, "podman inspect --format '{{.Id}}' busybox-with-tty").strip()
@@ -1567,7 +1567,7 @@ class TestApplication(testlib.MachineCase):
         # Run without tty, console should be able to `exec`
         b.set_checked("#run-image-dialog-tty", False)
 
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         b.wait_not_present("div.pf-c-modal-box")
 
         self.waitContainerRow("busybox-without-publish")
@@ -1650,9 +1650,7 @@ class TestApplication(testlib.MachineCase):
         b.wait_val("#create-image-image-select-typeahead", "quay.io/libpod/busybox:latest")
         b.set_input_text("#run-image-dialog-name", container_name)
 
-        # Uncheck start container after creation
-        b.click("#run-image-dialog-start-after-creation")
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-btn')
         b.wait_not_present("div.pf-c-modal-box")
 
         b.wait_val("#containers-containers-filter", "all")
@@ -1811,12 +1809,12 @@ class TestApplication(testlib.MachineCase):
         b.click('.publish-port-form .btn-add')
         b.set_input_text('#run-image-dialog-publish-0-host-port', '5000')
         b.set_input_text('#run-image-dialog-publish-0-container-port', '5000')
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         b.wait_in_text(".pf-c-alert", "address already in use")
 
         # Changing the port should allow creation of container
         b.set_input_text('#run-image-dialog-publish-0-host-port', '5001')
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         self.waitContainerRow(container_name)
 
     @testlib.skipImage("podman-restart not available in debian/ubuntu", *DISTROS_WITHOUT_PODMAN_RESTART)
@@ -1839,7 +1837,7 @@ class TestApplication(testlib.MachineCase):
             b.set_input_text("#run-image-dialog-name", name)
             if policy:
                 b.set_val("#run-image-dialog-restart-policy", "always")
-            b.click('.pf-c-modal-box__footer button:contains("Create")')
+            b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
             self.waitContainerRow(name)
 
         container_name = 'none'
@@ -1873,7 +1871,7 @@ class TestApplication(testlib.MachineCase):
         b.set_input_text("#create-image-image-select-typeahead", "quay.io/libpod/busybox:latest")
         b.click('button.pf-c-toggle-group__button:contains("Local")')
         b.click('button.pf-c-select__menu-item:contains("quay.io/libpod/busybox:latest")')
-        b.click('.pf-c-modal-box__footer button:contains("Create")')
+        b.click('.pf-c-modal-box__footer #create-image-create-run-btn')
         b.wait_not_present("#run-image-dialog-name")
 
         container_sha = self.execute(auth, f"podman inspect --format '{{{{.Id}}}}' {container_name}").strip()

diff --git a/test/reference b/test/reference
+ −		TestApplication-testCreateContainerSystem-image-select-medium-pixels.png
+ −		TestApplication-testCreateContainerSystem-image-select-mobile-pixels.png
+ −		TestApplication-testCreateContainerSystem-image-select-pixels.png
+ −		TestApplication-testRunImageSystem-integration-medium-pixels.png
+ −		TestApplication-testRunImageSystem-integration-mobile-pixels.png
+ −		TestApplication-testRunImageSystem-integration-pixels.png