fix(form): use icons for form buttons

* fix(form): switch to icon buttons * refactor: create TooltipButton * test(form): fix integration test * fix: add icon to TooltipButton
equinor · Sep 15, 2023 · 56bcd79 · 56bcd79
1 parent 5024217
commit 56bcd79
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 121 deletions.
diff --git a/e2e/tests/plugin-form-Nested.spec.ts b/e2e/tests/plugin-form-Nested.spec.ts
@@ -27,10 +27,7 @@ const navigate = async () => {
 
 test('Change owner', async () => {
   await expect(page.getByText('Owner', { exact: true })).toBeVisible
-  await page
-    .getByText('OwnerOpen')
-    .getByRole('button', { name: 'Open' })
-    .click()
+  await page.getByTestId('owner').getByRole('button', { name: 'Open' }).click()
   await page.getByLabel('Name').fill('Jacob')
   await page.getByLabel('Phone Number (optional)').fill('1234')
   await page.getByRole('button', { name: 'Submit' }).click()
@@ -156,10 +153,8 @@ test('New car', async () => {
 })
 
 test('New customer', async () => {
-  await page
-    .getByText('CustomersOpen')
-    .getByRole('button', { name: 'Open' })
-    .click()
+  const customersDiv = page.getByTestId('customers')
+  await customersDiv.getByRole('button', { name: 'Open' }).click()
   const lastTabPanel = page.getByRole('tabpanel').last()
   await expect(lastTabPanel).toBeVisible()
   await expect.soft(lastTabPanel.getByText('1 - 2 of 2')).toBeVisible()
@@ -174,10 +169,7 @@ test('New customer', async () => {
   await page.getByRole('button', { name: 'close', exact: true }).click()
   await page.reload()
   await navigate()
-  await page
-    .getByText('CustomersOpen')
-    .getByRole('button', { name: 'Open' })
-    .click()
+  await customersDiv.getByRole('button', { name: 'Open' }).click()
   await expect(page.getByText('Lewis')).toBeVisible()
   await page.getByRole('button', { name: 'Open item' }).last().click()
   await expect(page.getByRole('tab', { name: 'Lewis' })).toBeVisible()

diff --git a/e2e/tests/plugin-form-UncontainedObject.spec.ts b/e2e/tests/plugin-form-UncontainedObject.spec.ts
@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test'
+import { expect, test } from '@playwright/test'
 
 test('uncontainedObject', async ({ page }) => {
   await page.goto('http://localhost:3000/')
@@ -7,13 +7,6 @@ test('uncontainedObject', async ({ page }) => {
   await page.getByText('uncontained_object', { exact: true }).click()
   await page.getByText('UncontainedObject').click()
 
-  await expect(
-    page.getByTestId('ceo').getByText('Address: ^.employees[0]')
-  ).toBeVisible()
-  await expect(
-    page.getByTestId('accountant').getByText('Address: ^.employees[0]')
-  ).toBeVisible()
-
   await page.getByTestId('ceo').getByRole('button', { name: 'Open' }).click()
   await expect(page.getByRole('code').getByText('Miranda')).toBeVisible()
   await expect(page.getByRole('code').getByText('1337')).toBeVisible()
@@ -27,7 +20,7 @@ test('uncontainedObject', async ({ page }) => {
   // Add assistant
   await page
     .getByTestId('assistant')
-    .getByRole('button', { name: 'Select and save' })
+    .getByRole('button', { name: 'Add and save' })
     .click()
   await expect(page.getByRole('dialog')).toBeVisible()
   await page.getByRole('dialog').getByText('plugins', { exact: true }).click()
@@ -56,7 +49,7 @@ test('uncontainedObject', async ({ page }) => {
   // Add trainee
   await page
     .getByTestId('trainee')
-    .getByRole('button', { name: 'Select and save' })
+    .getByRole('button', { name: 'Add and save' })
     .click()
   await expect(page.getByRole('dialog')).toBeVisible()
   await page.getByRole('dialog').getByText('plugins', { exact: true }).click()
@@ -84,7 +77,7 @@ test('uncontainedObject', async ({ page }) => {
   // Change accountant
   await page
     .getByTestId('accountant')
-    .getByRole('button', { name: 'Select and save' })
+    .getByRole('button', { name: 'Edit and save' })
     .click()
   await expect(page.getByRole('dialog')).toBeVisible()
   await page.getByRole('dialog').getByText('plugins', { exact: true }).click()
@@ -109,13 +102,10 @@ test('uncontainedObject', async ({ page }) => {
   ).toBeVisible()
 
   //Remove trainee
-  await page.getByTestId('remove-trainee').click()
-  await expect(
-    page.getByTestId('trainee').getByRole('code').getByText('John')
-  ).not.toBeVisible()
-  await expect(
-    page.getByTestId('trainee').getByRole('code').getByText('1234')
-  ).not.toBeVisible()
+  const trainee = page.getByTestId('trainee')
+  await trainee.getByRole('button', { name: 'Remove and save' }).click()
+  await expect(trainee.getByRole('code').getByText('John')).not.toBeVisible()
+  await expect(trainee.getByRole('code').getByText('1234')).not.toBeVisible()
 
   // Submit form
   await page.getByRole('button', { name: 'Submit' }).click()

diff --git a/e2e/tests/plugin-view_selector-car_garage.spec.ts b/e2e/tests/plugin-view_selector-car_garage.spec.ts
@@ -1,4 +1,4 @@
-import { test, expect } from '@playwright/test'
+import { expect, test } from '@playwright/test'
 
 test('View selector - car garage', async ({ page }) => {
   // Open self and verify page content and the sidebar:
@@ -30,10 +30,7 @@ test('View selector - car garage', async ({ page }) => {
   await page.locator('a').filter({ hasText: 'Audi' }).click()
   await expect(page.getByRole('tab', { name: 'home Home' })).toBeVisible()
   await expect(page.getByRole('tabpanel').locator('#name')).toHaveValue('Audi')
-  await page
-    .getByText('View owner details and historyOpen')
-    .getByRole('button', { name: 'Open' })
-    .click()
+  await page.getByTestId('Owner').getByRole('button', { name: 'Open' }).click()
   await expect(
     page.getByRole('tab', { name: 'person Owner details' })
   ).toHaveAttribute('aria-selected', 'true')
@@ -49,7 +46,7 @@ test('View selector - car garage', async ({ page }) => {
   ).toHaveAttribute('aria-selected', 'true')
 
   //Add earlier owner:
-  await page.getByTestId('add-earlierOwners').click()
+  await page.getByRole('button', { name: 'Add' }).click()
   await page.getByRole('textbox').last().fill(' Joanna')
   await page.getByRole('button', { name: 'Submit' }).click()
   await expect(page.getByRole('alert')).toHaveText(['Document updated'])
@@ -58,7 +55,7 @@ test('View selector - car garage', async ({ page }) => {
   // Select "home" and then open "technical". Verify that both owner and techincal tabs are open and selectable. Check also sub-tabs
   await page.getByRole('tab', { name: 'home Home' }).click()
   await page
-    .getByText('View technical informationOpen')
+    .getByTestId('Technical')
     .getByRole('button', { name: 'Open' })
     .click()
   await expect(
@@ -113,7 +110,7 @@ test('View selector - car garage', async ({ page }) => {
   // Testing that saving one car does not ovveride the other car:
   await page.locator('a').filter({ hasText: 'Volvo' }).click()
   await page
-    .getByText('View technical informationOpen')
+    .getByTestId('Technical')
     .getByRole('button', { name: 'Open' })
     .click()
 
@@ -125,10 +122,7 @@ test('View selector - car garage', async ({ page }) => {
     '4500'
   )
   await page.getByRole('tab', { name: 'home Home' }).click()
-  await page
-    .getByText('View owner details and historyOpen')
-    .getByRole('button', { name: 'Open' })
-    .click()
+  await page.getByTestId('Owner').getByRole('button', { name: 'Open' }).click()
   await page.getByRole('tab', { name: 'group Owner history' }).click()
   await expect(page.getByRole('textbox').first()).toHaveValue('Jack')
   await expect(page.getByRole('textbox').last()).toHaveValue('Maria')

diff --git a/packages/dm-core-plugins/src/common/TooltipButton.tsx b/packages/dm-core-plugins/src/common/TooltipButton.tsx
@@ -0,0 +1,51 @@
+import { Button, Icon, Tooltip } from '@equinor/eds-core-react'
+import { IconData } from '@equinor/eds-icons'
+import React from 'react'
+
+type Prefix<T, P extends string> = {
+  [K in keyof T as `${P}-${string & K}`]: T[K]
+}
+type PrefixedButton = Prefix<
+  Omit<React.ComponentProps<typeof Button>, 'aria-label' | 'children'>,
+  'button'
+>
+type PrefixedTooltip = Prefix<
+  Omit<React.ComponentProps<typeof Tooltip>, 'title' | 'children'>,
+  'tooltip'
+>
+type TContent =
+  | {
+      icon?: IconData
+      buttonText: string
+    }
+  | {
+      icon: IconData
+      buttonText?: string
+    }
+type TProps = PrefixedButton & PrefixedTooltip & TContent & { title: string }
+
+const getProps = (prefix: string, dict: { [k: string]: any }) => {
+  return Object.fromEntries(
+    Object.entries(dict)
+      .filter(([k]) => k.startsWith(prefix))
+      .map(([k, v]) => [k.slice(prefix.length), v])
+  )
+}
+
+/**
+ * Tests can access the components through getByRole('button', { name: title })) or getByLabel(title)
+ * @param props Component accepts all props used by EDS Button and EDS Tooltip. However, to avoid interfering with each other, you'll have to prefix the prop with "button-" or "tooltip-". Ex: button-onChange. In addition, it has a mandatory title prop, which is used both as aria-label and tooltip title. You must also supply it with a buttonText and/or an icon.
+ * @returns An EDS button with EDS tooltip.
+ */
+const TooltipButton = (props: TProps) => {
+  return (
+    <Tooltip title={props.title} {...getProps('tooltip-', props)}>
+      <Button {...getProps('button-', props)} aria-label={props.title}>
+        {props.icon && <Icon data={props.icon} />}
+        {props.buttonText}
+      </Button>
+    </Tooltip>
+  )
+}
+
+export default TooltipButton
diff --git a/packages/dm-core-plugins/src/form/components/OpenObjectButton.tsx b/packages/dm-core-plugins/src/form/components/OpenObjectButton.tsx
@@ -3,8 +3,9 @@ import {
   TReferenceViewConfig,
   TViewConfig,
 } from '@development-framework/dm-core'
-import { Button } from '@equinor/eds-core-react'
+import { external_link } from '@equinor/eds-icons'
 import React from 'react'
+import TooltipButton from '../../common/TooltipButton'
 import { useRegistryContext } from '../context/RegistryContext'
 
 export const OpenObjectButton = ({
@@ -19,11 +20,11 @@ export const OpenObjectButton = ({
   const { onOpen } = useRegistryContext()
 
   return (
-    <Button
-      variant="outlined"
-      onClick={() => onOpen?.(viewId, view, idReference)}
-    >
-      Open
-    </Button>
+    <TooltipButton
+      title="Open in tab"
+      button-variant="ghost_icon"
+      button-onClick={() => onOpen?.(viewId, view, idReference)}
+      icon={external_link}
+    />
   )
 }
diff --git a/packages/dm-core-plugins/src/form/fields/ArrayField.tsx b/packages/dm-core-plugins/src/form/fields/ArrayField.tsx
@@ -6,13 +6,16 @@ import {
   getKey,
   useDMSS,
 } from '@development-framework/dm-core'
-import { Button, Typography } from '@equinor/eds-core-react'
+import { Typography } from '@equinor/eds-core-react'
 import { AxiosError } from 'axios'
 import React from 'react'
 import { useFieldArray, useFormContext } from 'react-hook-form'
 
+import { add, delete_forever } from '@equinor/eds-icons'
+import TooltipButton from '../../common/TooltipButton'
 import { OpenObjectButton } from '../components/OpenObjectButton'
 import { useRegistryContext } from '../context/RegistryContext'
+import { ButtonRow } from '../styles'
 import { TArrayFieldProps } from '../types'
 import { isPrimitive } from '../utils'
 import { AttributeField } from './AttributeField'
@@ -59,7 +62,7 @@ export default function ArrayField(props: TArrayFieldProps) {
 
   if (onOpen && !uiAttribute?.showInline && !isPrimitiveType(type)) {
     return (
-      <Stack spacing={0.25} alignItems="flex-start">
+      <ButtonRow>
         <Typography bold={true}>{displayLabel}</Typography>
         <OpenObjectButton
           viewId={namePath}
@@ -69,7 +72,7 @@ export default function ArrayField(props: TArrayFieldProps) {
             recipe: uiRecipeName,
           }}
         />
-      </Stack>
+      </ButtonRow>
     )
   }
 
@@ -90,14 +93,32 @@ export default function ArrayField(props: TArrayFieldProps) {
 
   return (
     <Stack spacing={0.5} alignItems="flex-start">
-      <Typography bold={true}>{displayLabel}</Typography>
+      <ButtonRow>
+        <Typography bold={true}>{displayLabel}</Typography>
+        {!readOnly && (
+          <TooltipButton
+            title="Add"
+            button-variant="ghost_icon"
+            button-onClick={() => {
+              if (isPrimitiveType(type)) {
+                const defaultValue = isPrimitive(type) ? ' ' : {}
+                append(defaultValue)
+              } else {
+                handleAddObject()
+              }
+            }}
+            icon={add}
+          />
+        )}
+      </ButtonRow>
       {fields.map((item: any, index: number) => {
         return (
           <Stack
             key={item.id}
             direction="row"
             spacing={0.5}
             alignSelf="stretch"
+            alignItems="flex-end"
           >
             <Stack grow={1}>
               <AttributeField
@@ -112,33 +133,16 @@ export default function ArrayField(props: TArrayFieldProps) {
               />
             </Stack>
             {!readOnly && (
-              <Button
-                variant="outlined"
-                type="button"
-                onClick={() => remove(index)}
-              >
-                Remove
-              </Button>
+              <TooltipButton
+                title="Remove"
+                button-variant="ghost_icon"
+                button-onClick={() => remove(index)}
+                icon={delete_forever}
+              />
             )}
           </Stack>
         )
       })}
-      {!readOnly && (
-        <Button
-          variant="outlined"
-          data-testid={`add-${namePath}`}
-          onClick={() => {
-            if (isPrimitiveType(type)) {
-              const defaultValue = isPrimitive(type) ? ' ' : {}
-              append(defaultValue)
-            } else {
-              handleAddObject()
-            }
-          }}
-        >
-          Add
-        </Button>
-      )}
     </Stack>
   )
 }
diff --git a/packages/dm-core-plugins/src/form/fields/ObjectField.test.tsx b/packages/dm-core-plugins/src/form/fields/ObjectField.test.tsx
@@ -138,6 +138,6 @@ test('should handle optional', async () => {
     // Show optional in label
     expect(screen.getByText('nested (optional)')).toBeDefined()
     // Add button
-    expect(screen.getByTestId('add-nested')).toBeDefined()
+    expect(screen.getByRole('button', { name: 'Add and save' })).toBeDefined()
   })
 })