Modals: Be more consistent with Modal cancel button styling (#68302)

* Modals: Be more consistent with Modal cancel button styling

* Update docs

* Fix tests

* fixing tests

packages/grafana-ui/src/components/ConfirmModal/ConfirmModal.tsx

@@ -97,7 +97,7 @@ export const ConfirmModal = ({
         ) : null}
       </div>
       <Modal.ButtonRow>
-        <Button variant={dismissVariant} onClick={onDismiss}>
+        <Button variant={dismissVariant} onClick={onDismiss} fill="outline">
           {dismissText}
         </Button>
         <Button

packages/grafana-ui/src/components/Modal/Modal.mdx

@@ -6,7 +6,26 @@ import { ModalTabsHeader } from './ModalTabsHeader';
 
 # Modal
 
-Generic Modal component
+Generic Modal component.
+
+Example usage:
+
+```tsx
+<Modal title="title" isOpen={true}>
+  <div>Some body</div>
+  <Modal.ButtonRow>
+    <Button variant="secondary" fill="outline">
+      Cancel
+    </Button>
+    <Button>Save</Button>
+  </Modal.ButtonRow>
+</Modal>
+```
+
+For buttons at the bottom of the modal you should always use:
+
+- Modal.ButtonRow - this makes sure the Buttons are right aligned
+- If you have a Cancel button use fill="outline" and variant="secondary" and place it on the left side of any other button.
 
 <ArgTypes of={Modal} />
 

public/app/features/alerting/unified/components/notification-policies/Modals.tsx

@@ -62,10 +62,10 @@ const useAddPolicyModal = (
             onSubmit={(newRoute) => parentRoute && handleAdd(newRoute, parentRoute)}
             actionButtons={
               <Modal.ButtonRow>
-                <Button type="submit">Save policy</Button>
-                <Button type="button" variant="secondary" onClick={handleDismiss}>
+                <Button type="button" variant="secondary" onClick={handleDismiss} fill="outline">
                   Cancel
                 </Button>
+                <Button type="submit">Save policy</Button>
               </Modal.ButtonRow>
             }
           />
@@ -121,10 +121,10 @@ const useEditPolicyModal = (
               route={route}
               actionButtons={
                 <Modal.ButtonRow>
-                  <Button type="submit">Update default policy</Button>
-                  <Button type="button" variant="secondary" onClick={handleDismiss}>
+                  <Button type="button" variant="secondary" onClick={handleDismiss} fill="outline">
                     Cancel
                   </Button>
+                  <Button type="submit">Update default policy</Button>
                 </Modal.ButtonRow>
               }
             />
@@ -136,10 +136,10 @@ const useEditPolicyModal = (
               onSubmit={handleSave}
               actionButtons={
                 <Modal.ButtonRow>
-                  <Button type="submit">Update policy</Button>
-                  <Button type="button" variant="secondary" onClick={handleDismiss}>
+                  <Button type="button" variant="secondary" onClick={handleDismiss} fill="outline">
                     Cancel
                   </Button>
+                  <Button type="submit">Update policy</Button>
                 </Modal.ButtonRow>
               }
             />

public/app/features/alerting/unified/components/rule-editor/DashboardPicker.tsx

@@ -230,6 +230,9 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
         </div>
       </div>
       <Modal.ButtonRow>
+        <Button type="button" variant="secondary" onClick={onDismiss} fill="text">
+          Cancel
+        </Button>
         <Button
           type="button"
           variant="primary"
@@ -242,9 +245,6 @@ export const DashboardPicker = ({ dashboardUid, panelId, isOpen, onChange, onDis
         >
           Confirm
         </Button>
-        <Button type="button" variant="secondary" onClick={onDismiss}>
-          Cancel
-        </Button>
       </Modal.ButtonRow>
     </Modal>
   );

public/app/features/alerting/unified/components/rules/EditRuleGroupModal.tsx

@@ -374,6 +374,15 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
             )}
             <div className={styles.modalButtons}>
               <Modal.ButtonRow>
+                <Button
+                  variant="secondary"
+                  type="button"
+                  disabled={loading}
+                  onClick={() => onClose(false)}
+                  fill="outline"
+                >
+                  Cancel
+                </Button>
                 <Button
                   type="button"
                   disabled={!isDirty || loading}
@@ -381,9 +390,6 @@ export function EditCloudGroupModal(props: ModalProps): React.ReactElement {
                 >
                   {loading ? 'Saving...' : 'Save evaluation interval'}
                 </Button>
-                <Button variant="secondary" type="button" disabled={loading} onClick={() => onClose(false)}>
-                  Cancel
-                </Button>
               </Modal.ButtonRow>
             </div>
             {!hasSomeNoRecordingRules && <div>This group does not contain alert rules.</div>}

public/app/features/browse-dashboards/components/BrowseActions/MoveModal.tsx

@@ -45,7 +45,7 @@ export const MoveModal = ({ onConfirm, onDismiss, selectedItems, ...props }: Pro
         <FolderPicker allowEmpty onChange={({ uid }) => setMoveTarget(uid)} />
       </Field>
       <Modal.ButtonRow>
-        <Button onClick={onDismiss} variant="secondary">
+        <Button onClick={onDismiss} variant="secondary" fill="outline">
           Cancel
         </Button>
         <Button disabled={moveTarget === undefined} onClick={onMove} variant="primary">

public/app/features/dimensions/editors/ResourcePickerPopover.tsx

@@ -99,7 +99,7 @@ export const ResourcePickerPopover = (props: Props) => {
           <div className={styles.resourcePickerPopoverContent}>
             {renderPicker()}
             <ButtonGroup className={styles.buttonGroup}>
-              <Button className={styles.button} variant={'secondary'} onClick={() => onClose()}>
+              <Button className={styles.button} variant={'secondary'} onClick={() => onClose()} fill="outline">
                 Cancel
               </Button>
               <Button

public/app/features/search/page/components/MoveToFolderModal.tsx

@@ -129,12 +129,12 @@ export const MoveToFolderModal = ({ results, onMoveItems, onDismiss }: Props) =>
         </div>
 
         <HorizontalGroup justify="flex-end">
+          <Button variant="secondary" onClick={onDismiss} fill="outline">
+            Cancel
+          </Button>
           <Button icon={moving ? 'fa fa-spinner' : undefined} variant="primary" onClick={moveTo}>
             Move
           </Button>
-          <Button variant="secondary" onClick={onDismiss}>
-            Cancel
-          </Button>
         </HorizontalGroup>
       </>
     </Modal>

public/app/plugins/datasource/azuremonitor/components/ResourcePicker/ResourcePicker.tsx

@@ -2,7 +2,7 @@ import { cx } from '@emotion/css';
 import React, { useCallback, useEffect, useState } from 'react';
 import { useEffectOnce } from 'react-use';
 
-import { Alert, Button, LoadingPlaceholder, useStyles2 } from '@grafana/ui';
+import { Alert, Button, LoadingPlaceholder, Modal, useStyles2 } from '@grafana/ui';
 
 import { selectors } from '../../e2e/selectors';
 import ResourcePickerData, { ResourcePickerQueryType } from '../../resourcePicker/resourcePickerData';
@@ -261,31 +261,28 @@ const ResourcePicker = ({
           renderAdvanced={renderAdvanced}
         />
 
-        <Space v={2} />
+        {errorMessage && (
+          <>
+            <Space v={2} />
+            <Alert severity="error" title="An error occurred while requesting resources from Azure Monitor">
+              {errorMessage}
+            </Alert>
+          </>
+        )}
 
-        <Button
-          disabled={!!errorMessage || !internalSelected.every(isValid)}
-          onClick={handleApply}
-          data-testid={selectors.components.queryEditor.resourcePicker.apply.button}
-        >
-          Apply
-        </Button>
-
-        <Space layout="inline" h={1} />
-
-        <Button onClick={onCancel} variant="secondary">
-          Cancel
-        </Button>
+        <Modal.ButtonRow>
+          <Button onClick={onCancel} variant="secondary" fill="outline">
+            Cancel
+          </Button>
+          <Button
+            disabled={!!errorMessage || !internalSelected.every(isValid)}
+            onClick={handleApply}
+            data-testid={selectors.components.queryEditor.resourcePicker.apply.button}
+          >
+            Apply
+          </Button>
+        </Modal.ButtonRow>
       </div>
-
-      {errorMessage && (
-        <>
-          <Space v={2} />
-          <Alert severity="error" title="An error occurred while requesting resources from Azure Monitor">
-            {errorMessage}
-          </Alert>
-        </>
-      )}
     </div>
   );
 };

public/app/plugins/datasource/cloudwatch/components/ConfigEditor.test.tsx

@@ -150,7 +150,7 @@ describe('Render', () => {
 
   it('should display log group selector field', async () => {
     setup();
-    await waitFor(async () => expect(await screen.getByText('Select Log Groups')).toBeInTheDocument());
+    await waitFor(async () => expect(await screen.getByText('Select log groups')).toBeInTheDocument());
   });
 
   it('should only display the first two default log groups and show all of them when clicking "Show all" button', async () => {
@@ -215,8 +215,8 @@ describe('Render', () => {
 
     render(<ConfigEditor {...newProps} />);
 
-    await waitFor(() => expect(screen.getByText('Select Log Groups')).toBeInTheDocument());
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await waitFor(() => expect(screen.getByText('Select log groups')).toBeInTheDocument());
+    await userEvent.click(screen.getByText('Select log groups'));
     await waitFor(() =>
       expect(screen.getByText('You need to save the data source before adding log groups.')).toBeInTheDocument()
     );
@@ -232,7 +232,7 @@ describe('Render', () => {
       },
     };
     const { rerender } = render(<ConfigEditor {...newProps} />);
-    await waitFor(() => expect(screen.getByText('Select Log Groups')).toBeInTheDocument());
+    await waitFor(() => expect(screen.getByText('Select log groups')).toBeInTheDocument());
     const rerenderProps = {
       ...newProps,
       options: {
@@ -245,7 +245,7 @@ describe('Render', () => {
     };
     rerender(<ConfigEditor {...rerenderProps} />);
     await waitFor(() => expect(screen.getByText('AWS SDK Default')).toBeInTheDocument());
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     await waitFor(() =>
       expect(
         screen.getByText(
@@ -265,7 +265,7 @@ describe('Render', () => {
       },
     };
     const { rerender } = render(<ConfigEditor {...newProps} />);
-    await waitFor(() => expect(screen.getByText('Select Log Groups')).toBeInTheDocument());
+    await waitFor(() => expect(screen.getByText('Select log groups')).toBeInTheDocument());
     const rerenderProps = {
       ...newProps,
       options: {
@@ -274,7 +274,7 @@ describe('Render', () => {
       },
     };
     rerender(<ConfigEditor {...rerenderProps} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     await waitFor(() => expect(screen.getByText('Log group name prefix')).toBeInTheDocument());
   });
 });

public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsField.test.tsx

@@ -37,7 +37,7 @@ describe('LogGroupSelection', () => {
       .mockResolvedValue([{ value: { arn: 'arn', name: 'loggroupname' } }]);
     render(<LogGroupsField {...defaultProps} legacyLogGroupNames={['loggroupname']} />);
 
-    await waitFor(async () => expect(screen.getByText('Select Log Groups')).toBeInTheDocument());
+    await waitFor(async () => expect(screen.getByText('Select log groups')).toBeInTheDocument());
     expect(defaultProps.datasource.resources.getLogGroups).toHaveBeenCalledWith({
       region: defaultProps.region,
       logGroupNamePrefix: 'loggroupname',
@@ -53,7 +53,7 @@ describe('LogGroupSelection', () => {
       .mockResolvedValue([{ value: { arn: 'arn', name: 'loggroupname' } }]);
     render(<LogGroupsField {...defaultProps} legacyLogGroupNames={['loggroupname', logGroupNamesVariable.name]} />);
 
-    await waitFor(async () => expect(screen.getByText('Select Log Groups')).toBeInTheDocument());
+    await waitFor(async () => expect(screen.getByText('Select log groups')).toBeInTheDocument());
     expect(defaultProps.datasource.resources.getLogGroups).toHaveBeenCalledTimes(1);
     expect(defaultProps.datasource.resources.getLogGroups).toHaveBeenCalledWith({
       region: defaultProps.region,
@@ -71,7 +71,7 @@ describe('LogGroupSelection', () => {
       .fn()
       .mockResolvedValue([{ value: { arn: 'arn', name: 'loggroupname' } }]);
     render(<LogGroupsField {...defaultProps} logGroups={[{ arn: 'arn', name: 'loggroupname' }]} />);
-    await waitFor(() => expect(screen.getByText('Select Log Groups')).toBeInTheDocument());
+    await waitFor(() => expect(screen.getByText('Select log groups')).toBeInTheDocument());
     expect(defaultProps.datasource.resources.getLogGroups).not.toHaveBeenCalled();
     expect(defaultProps.onChange).not.toHaveBeenCalled();
   });

public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsSelector.test.tsx

@@ -68,9 +68,9 @@ describe('LogGroupsSelector', () => {
   afterEach(() => {
     lodash.debounce = originalDebounce;
   });
-  it('opens a modal with a search field when the Select Log Groups Button is clicked', async () => {
+  it('opens a modal with a search field when the Select log groups Button is clicked', async () => {
     render(<LogGroupsSelector {...defaultProps} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
   });
 
@@ -81,7 +81,7 @@ describe('LogGroupsSelector', () => {
       return defaultProps.fetchLogGroups();
     });
     render(<LogGroupsSelector {...defaultProps} fetchLogGroups={fetchLogGroups} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Loading...')).toBeInTheDocument();
     defer.resolve();
     await waitFor(() => expect(screen.queryByText('Loading...')).not.toBeInTheDocument());
@@ -96,7 +96,7 @@ describe('LogGroupsSelector', () => {
       return [];
     });
     render(<LogGroupsSelector {...defaultProps} fetchLogGroups={fetchLogGroups} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Loading...')).toBeInTheDocument();
     defer.resolve();
     await waitFor(() => expect(screen.queryByText('Loading...')).not.toBeInTheDocument());
@@ -108,7 +108,7 @@ describe('LogGroupsSelector', () => {
   it('calls fetchLogGroups with a search phrase when it is typed in the Search Field', async () => {
     const fetchLogGroups = jest.fn(() => defaultProps.fetchLogGroups());
     render(<LogGroupsSelector {...defaultProps} fetchLogGroups={fetchLogGroups} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
     await userEvent.type(screen.getByLabelText('log group search'), 'something');
     await waitFor(() => screen.getByDisplayValue('something'));
@@ -129,7 +129,7 @@ describe('LogGroupsSelector', () => {
       return defaultProps.fetchLogGroups();
     });
     render(<LogGroupsSelector {...defaultProps} fetchLogGroups={fetchLogGroups} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Loading...')).toBeInTheDocument();
     firstCall.resolve();
     await waitFor(() => expect(screen.queryByText('Loading...')).not.toBeInTheDocument());
@@ -147,7 +147,7 @@ describe('LogGroupsSelector', () => {
   it('shows a log group as checked after the user checks it', async () => {
     const onChange = jest.fn();
     render(<LogGroupsSelector {...defaultProps} onChange={onChange} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
     expect(screen.getByLabelText('logGroup2')).not.toBeChecked();
     await userEvent.click(screen.getByLabelText('logGroup2'));
@@ -157,7 +157,7 @@ describe('LogGroupsSelector', () => {
   it('calls onChange with the selected log group when checked and the user clicks the Add button', async () => {
     const onChange = jest.fn();
     render(<LogGroupsSelector {...defaultProps} onChange={onChange} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
     await userEvent.click(screen.getByLabelText('logGroup2'));
     await userEvent.click(screen.getByText('Add log groups'));
@@ -174,7 +174,7 @@ describe('LogGroupsSelector', () => {
   it('does not call onChange after a selection if the user hits the cancel button', async () => {
     const onChange = jest.fn();
     render(<LogGroupsSelector {...defaultProps} onChange={onChange} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
     await userEvent.click(screen.getByLabelText('logGroup2'));
     await userEvent.click(screen.getByText('Cancel'));
@@ -197,7 +197,7 @@ describe('LogGroupsSelector', () => {
       return [];
     });
     render(<LogGroupsSelector {...defaultProps} fetchLogGroups={fetchLogGroups} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.queryByText(labelText)).not.toBeInTheDocument();
     defer.resolve();
     await waitFor(() => expect(screen.queryByText(labelText)).not.toBeInTheDocument());
@@ -215,7 +215,7 @@ describe('LogGroupsSelector', () => {
       }));
     });
     render(<LogGroupsSelector {...defaultProps} fetchLogGroups={fetchLogGroups} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.queryByText(labelText)).not.toBeInTheDocument();
     defer.resolve();
     await waitFor(() => expect(screen.getByText(labelText)).toBeInTheDocument());
@@ -223,7 +223,7 @@ describe('LogGroupsSelector', () => {
 
   it('should display log groups counter label', async () => {
     render(<LogGroupsSelector {...defaultProps} selectedLogGroups={[]} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     await waitFor(() => expect(screen.getByText('0 log groups selected')).toBeInTheDocument());
     await userEvent.click(screen.getByLabelText('logGroup2'));
     await waitFor(() => expect(screen.getByText('1 log group selected')).toBeInTheDocument());
@@ -239,7 +239,7 @@ describe('LogGroupsSelector', () => {
         ]}
       />
     );
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     await waitFor(() => expect(screen.getByText('1 log group selected')).toBeInTheDocument());
   });
 
@@ -255,7 +255,7 @@ describe('LogGroupsSelector', () => {
         onChange={onChange}
       />
     );
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     await selectEvent.select(screen.getByLabelText('Template variable'), '$logGroupVariable', {
       container: document.body,
     });
@@ -284,7 +284,7 @@ describe('LogGroupsSelector', () => {
         onChange={onChange}
       />
     );
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     await screen.getByRole('button', { name: 'select-clear-value' }).click();
     await userEvent.click(screen.getByText('Add log groups'));
     expect(onChange).toHaveBeenCalledWith([
@@ -297,7 +297,7 @@ describe('LogGroupsSelector', () => {
 
   it('should display account label if account options prop has values', async () => {
     render(<LogGroupsSelector {...defaultProps} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
     expect(screen.getByText('Account label')).toBeInTheDocument();
     waitFor(() => expect(screen.getByText('Account Name 123')).toBeInTheDocument());
@@ -305,7 +305,7 @@ describe('LogGroupsSelector', () => {
 
   it('should not display account label if account options prop doesnt has values', async () => {
     render(<LogGroupsSelector {...defaultProps} accountOptions={[]} />);
-    await userEvent.click(screen.getByText('Select Log Groups'));
+    await userEvent.click(screen.getByText('Select log groups'));
     expect(screen.getByText('Log group name prefix')).toBeInTheDocument();
     expect(screen.queryByText('Account label')).not.toBeInTheDocument();
     waitFor(() => expect(screen.queryByText('Account Name 123')).not.toBeInTheDocument());

public/app/plugins/datasource/cloudwatch/components/LogGroups/LogGroupsSelector.tsx

@@ -112,7 +112,7 @@ export const LogGroupsSelector = ({
 
   return (
     <>
-      <Modal className={styles.modal} title="Select Log Groups" isOpen={isModalOpen} onDismiss={toggleModal}>
+      <Modal className={styles.modal} title="Select log groups" isOpen={isModalOpen} onDismiss={toggleModal}>
         <div className={styles.logGroupSelectionArea}>
           <div className={styles.searchField}>
             <EditorField label="Log group name prefix">
@@ -229,15 +229,15 @@ export const LogGroupsSelector = ({
             }}
           />
         </EditorField>
-        <Space layout="block" v={2} />
-        <div>
-          <Button onClick={handleApply} type="button" className={styles.addBtn}>
-            Add log groups
-          </Button>
-          <Button onClick={handleCancel} variant="secondary" type="button">
+
+        <Modal.ButtonRow>
+          <Button onClick={handleCancel} variant="secondary" type="button" fill="outline">
             Cancel
           </Button>
-        </div>
+          <Button onClick={handleApply} type="button">
+            Add log groups
+          </Button>
+        </Modal.ButtonRow>
       </Modal>
 
       <div>
@@ -251,7 +251,7 @@ export const LogGroupsSelector = ({
           }}
           type="button"
         >
-          Select Log Groups
+          Select log groups
         </Button>
       </div>
     </>

public/app/plugins/datasource/cloudwatch/components/QueryEditor.test.tsx

@@ -118,7 +118,7 @@ describe('QueryEditor should render right editor', () => {
         statistic: 'Average',
       } as CloudWatchQuery;
       render(<QueryEditor {...props} query={query} />);
-      expect(await screen.findByText('Select Log Groups')).toBeInTheDocument();
+      expect(await screen.findByText('Select log groups')).toBeInTheDocument();
     });
   });
 

public/app/plugins/datasource/cloudwatch/components/styles.ts

@@ -95,10 +95,6 @@ const getStyles = (theme: GrafanaTheme2) => ({
     verticalAlign: 'middle',
     marginLeft: theme.spacing(0.5),
   }),
-
-  addBtn: css({
-    marginRight: '10px',
-  }),
 });
 
 export default getStyles;