PublicDashboards: UI improvements (#55130)

* Public dashboard modal UI modifications
2022-09-14 14:35:46 -03:00 · 2022-09-14 14:35:46 -03:00 · 1e06b0170b
parent 29327cbba2
commit 1e06b0170b
6 changed files with 184 additions and 141 deletions
--- a/packages/grafana-e2e-selectors/src/selectors/pages.ts
+++ b/packages/grafana-e2e-selectors/src/selectors/pages.ts
@ -181,7 +181,7 @@ export const Pages = {
  ShareDashboardModal: {
    shareButton: 'Share dashboard or panel',
    PublicDashboard: {
-      Tab: 'Tab Public Dashboard',
+      Tab: 'Tab Public dashboard',
      WillBePublicCheckbox: 'data-testid public dashboard will be public checkbox',
      LimitedDSCheckbox: 'data-testid public dashboard limited datasources checkbox',
      CostIncreaseCheckbox: 'data-testid public dashboard cost may increase checkbox',
--- a/public/app/features/dashboard/components/ShareModal/ShareModal.tsx
+++ b/public/app/features/dashboard/components/ShareModal/ShareModal.tsx
@ -62,7 +62,7 @@ function getTabs(props: Props) {
  }

  if (Boolean(config.featureToggles['publicDashboards'])) {
-    tabs.push({ label: 'Public Dashboard', value: 'share', component: SharePublicDashboard });
+    tabs.push({ label: 'Public dashboard', value: 'share', component: SharePublicDashboard });
  }

  return tabs;
--- a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard.test.tsx
+++ b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard.test.tsx
@ -74,7 +74,7 @@ describe('SharePublic', () => {
    render(<ShareModal panel={mockPanel} dashboard={mockDashboard} onDismiss={() => {}} />);

    expect(screen.getByRole('tablist')).toHaveTextContent('Link');
-    expect(screen.getByRole('tablist')).not.toHaveTextContent('Public Dashboard');
+    expect(screen.getByRole('tablist')).not.toHaveTextContent('Public dashboard');
  });

  it('renders share panel when public dashboards feature is enabled', async () => {
@ -90,14 +90,14 @@ describe('SharePublic', () => {

    await waitFor(() => screen.getByText('Link'));
    expect(screen.getByRole('tablist')).toHaveTextContent('Link');
-    expect(screen.getByRole('tablist')).toHaveTextContent('Public Dashboard');
+    expect(screen.getByRole('tablist')).toHaveTextContent('Public dashboard');

-    fireEvent.click(screen.getByText('Public Dashboard'));
+    fireEvent.click(screen.getByText('Public dashboard'));

    await screen.findByText('Welcome to Grafana public dashboards alpha!');
  });

-  it('renders default time in inputs', async () => {
+  it('renders default relative time in input', async () => {
    config.featureToggles.publicDashboards = true;
    const mockDashboard = new DashboardModel({
      uid: 'mockDashboardUid',
@ -107,17 +107,38 @@ describe('SharePublic', () => {
    });

    expect(mockDashboard.time).toEqual({ from: 'now-6h', to: 'now' });
+
    //@ts-ignore
-    mockDashboard.originalTime = { from: 'test-from', to: 'test-to' };
+    mockDashboard.originalTime = { from: 'now-6h', to: 'now' };

    render(<ShareModal panel={mockPanel} dashboard={mockDashboard} onDismiss={() => {}} />);

    await waitFor(() => screen.getByText('Link'));
-    fireEvent.click(screen.getByText('Public Dashboard'));
+    fireEvent.click(screen.getByText('Public dashboard'));

    await screen.findByText('Welcome to Grafana public dashboards alpha!');
-    expect(screen.getByDisplayValue('test-from')).toBeInTheDocument();
-    expect(screen.getByDisplayValue('test-to')).toBeInTheDocument();
+    expect(screen.getByText('Last 6 hours')).toBeInTheDocument();
+  });
+  it('renders default absolute time in input 2', async () => {
+    config.featureToggles.publicDashboards = true;
+    const mockDashboard = new DashboardModel({
+      uid: 'mockDashboardUid',
+    });
+    const mockPanel = new PanelModel({
+      id: 'mockPanelId',
+    });
+
+    mockDashboard.time = { from: '2022-08-30T03:00:00.000Z', to: '2022-09-04T02:59:59.000Z' };
+    //@ts-ignore
+    mockDashboard.originalTime = { from: '2022-08-30T06:00:00.000Z', to: '2022-09-04T06:59:59.000Z' };
+
+    render(<ShareModal panel={mockPanel} dashboard={mockDashboard} onDismiss={() => {}} />);
+
+    await waitFor(() => screen.getByText('Link'));
+    fireEvent.click(screen.getByText('Public dashboard'));
+
+    await screen.findByText('Welcome to Grafana public dashboards alpha!');
+    expect(screen.getByText('2022-08-30 00:00:00 to 2022-09-04 01:59:59')).toBeInTheDocument();
  });

  // test checking if current version of dashboard in state is persisted to db
--- a/public/app/features/dashboard/components/ShareModal/SharePublicDashboard.tsx
+++ b/public/app/features/dashboard/components/ShareModal/SharePublicDashboard.tsx
@ -1,5 +1,7 @@
+import { css } from '@emotion/css';
 import React, { useCallback, useEffect, useState } from 'react';

+import { GrafanaTheme2 } from '@grafana/data/src';
 import { selectors as e2eSelectors } from '@grafana/e2e-selectors/src';
 import { reportInteraction } from '@grafana/runtime/src';
 import {
@ -8,14 +10,19 @@ import {
  Checkbox,
  ClipboardButton,
  Field,
+  HorizontalGroup,
  FieldSet,
  Input,
  Label,
  LinkButton,
  Switch,
+  TimeRangeInput,
+  useStyles2,
+  VerticalGroup,
 } from '@grafana/ui';
 import { notifyApp } from 'app/core/actions';
 import { createErrorNotification } from 'app/core/copy/appNotification';
+import { getTimeRange } from 'app/features/dashboard/utils/timeRange';
 import { dispatch } from 'app/store/store';

 import { contextSrv } from '../../../../core/services/context_srv';
@ -43,6 +50,7 @@ interface Acknowledgements {
 export const SharePublicDashboard = (props: Props) => {
  const dashboardVariables = props.dashboard.getVariables();
  const selectors = e2eSelectors.pages.ShareDashboardModal.PublicDashboard;
+  const styles = useStyles2(getStyles);

  const hasWritePermissions = contextSrv.hasAccess(AccessControlAction.DashboardsPublicWrite, isOrgAdmin());

@ -57,6 +65,11 @@ export const SharePublicDashboard = (props: Props) => {
    usage: false,
  });

+  const timeRange = getTimeRange(
+    { from: props.dashboard.getDefaultTime().from, to: props.dashboard.getDefaultTime().to },
+    props.dashboard.timezone
+  );
+
  useEffect(() => {
    reportInteraction('grafana_dashboards_public_share_viewed');

@ -94,9 +107,7 @@ export const SharePublicDashboard = (props: Props) => {
  );

  // check if all conditions have been acknowledged
-  const acknowledged = () => {
-    return acknowledgements.public && acknowledgements.datasources && acknowledgements.usage;
-  };
+  const acknowledged = acknowledgements.public && acknowledgements.datasources && acknowledgements.usage;

  return (
    <>
@ -115,133 +126,116 @@ export const SharePublicDashboard = (props: Props) => {
            To allow the current dashboard to be published publicly, toggle the switch. For now we do not support
            template variables or frontend datasources.
          </p>
-          We&apos;d love your feedback. To share, please comment on this{' '}
-          <a
-            href="https://github.com/grafana/grafana/discussions/49253"
-            target="_blank"
-            rel="noreferrer"
-            className="text-link"
-          >
-            GitHub discussion
-          </a>
-          .
+          <p>
+            We&apos;d love your feedback. To share, please comment on this{' '}
+            <a
+              href="https://github.com/grafana/grafana/discussions/49253"
+              target="_blank"
+              rel="noreferrer"
+              className="text-link"
+            >
+              GitHub discussion
+            </a>
+            .
+          </p>
          <hr />
-          <div>
-            Before you click Save, please acknowledge the following information: <br />
+          <div className={styles.checkboxes}>
+            <p>Before you click Save, please acknowledge the following information:</p>
            <FieldSet disabled={publicDashboardPersisted(publicDashboard) || !hasWritePermissions}>
-              <br />
-              <div>
+              <VerticalGroup spacing="md">
                <Checkbox
                  label="Your entire dashboard will be public"
                  value={acknowledgements.public}
                  data-testid={selectors.WillBePublicCheckbox}
                  onChange={(e) => onAcknowledge('public', e.currentTarget.checked)}
                />
-              </div>
-              <br />
-              <div>
-                <Checkbox
-                  label="Publishing currently only works with a subset of datasources"
-                  value={acknowledgements.datasources}
-                  data-testid={selectors.LimitedDSCheckbox}
-                  onChange={(e) => onAcknowledge('datasources', e.currentTarget.checked)}
-                />
-                <LinkButton
-                  variant="primary"
-                  href="https://grafana.com/docs/grafana/latest/datasources/"
-                  target="_blank"
-                  fill="text"
-                  icon="info-circle"
-                  rel="noopener noreferrer"
-                  tooltip="Learn more about public datasources"
-                />
-              </div>
-              <br />
-              <Checkbox
-                label="Making your dashboard public will cause queries to run each time the dashboard is viewed which may increase costs"
-                value={acknowledgements.usage}
-                data-testid={selectors.CostIncreaseCheckbox}
-                onChange={(e) => onAcknowledge('usage', e.currentTarget.checked)}
-              />
-              <LinkButton
-                variant="primary"
-                href="https://grafana.com/docs/grafana/latest/enterprise/query-caching/"
-                target="_blank"
-                fill="text"
-                icon="info-circle"
-                rel="noopener noreferrer"
-                tooltip="Learn more about query caching"
-              />
-              <br />
-              <br />
+                <HorizontalGroup spacing="none">
+                  <Checkbox
+                    label="Publishing currently only works with a subset of datasources"
+                    value={acknowledgements.datasources}
+                    data-testid={selectors.LimitedDSCheckbox}
+                    onChange={(e) => onAcknowledge('datasources', e.currentTarget.checked)}
+                  />
+                  <LinkButton
+                    variant="primary"
+                    href="https://grafana.com/docs/grafana/latest/datasources/"
+                    target="_blank"
+                    fill="text"
+                    icon="info-circle"
+                    rel="noopener noreferrer"
+                    tooltip="Learn more about public datasources"
+                  />
+                </HorizontalGroup>
+                <HorizontalGroup spacing="none">
+                  <Checkbox
+                    label="Making your dashboard public will cause queries to run each time the dashboard is viewed which may increase costs"
+                    value={acknowledgements.usage}
+                    data-testid={selectors.CostIncreaseCheckbox}
+                    onChange={(e) => onAcknowledge('usage', e.currentTarget.checked)}
+                  />
+                  <LinkButton
+                    variant="primary"
+                    href="https://grafana.com/docs/grafana/latest/enterprise/query-caching/"
+                    target="_blank"
+                    fill="text"
+                    icon="info-circle"
+                    rel="noopener noreferrer"
+                    tooltip="Learn more about query caching"
+                  />
+                </HorizontalGroup>
+              </VerticalGroup>
            </FieldSet>
          </div>
+          <hr />
          <div>
-            <h4 className="share-modal-info-text">Public Dashboard Configuration</h4>
-            <FieldSet disabled={!hasWritePermissions}>
-              <Label description="The public dashboard uses the default time settings of the dashboard">
-                Time Range
-              </Label>
-              <div style={{ padding: '5px' }}>
-                <Input
-                  value={props.dashboard.getDefaultTime().from}
-                  disabled={true}
-                  addonBefore={
-                    <span style={{ width: '50px', display: 'flex', alignItems: 'center', padding: '5px' }}>From:</span>
-                  }
-                />
-                <Input
-                  value={props.dashboard.getDefaultTime().to}
-                  disabled={true}
-                  addonBefore={
-                    <span style={{ width: '50px', display: 'flex', alignItems: 'center', padding: '5px' }}>To:</span>
-                  }
-                />
-              </div>
-              <br />
-              <Field label="Enabled" description="Configures whether current dashboard can be available publicly">
-                <Switch
-                  disabled={dashboardHasTemplateVariables(dashboardVariables)}
-                  data-testid={selectors.EnableSwitch}
-                  value={publicDashboard?.isEnabled}
-                  onChange={() => {
-                    reportInteraction('grafana_dashboards_public_enable_clicked', {
-                      action: publicDashboard?.isEnabled ? 'disable' : 'enable',
-                    });
+            <h4 className="share-modal-info-text">Public dashboard configuration</h4>
+            <FieldSet disabled={!hasWritePermissions} className={styles.dashboardConfig}>
+              <VerticalGroup spacing="md">
+                <HorizontalGroup spacing="xs" justify="space-between">
+                  <Label description="The public dashboard uses the default time settings of the dashboard">
+                    Time Range
+                  </Label>
+                  <TimeRangeInput value={timeRange} disabled onChange={() => {}} />
+                </HorizontalGroup>
+                <HorizontalGroup spacing="xs" justify="space-between">
+                  <Label description="Configures whether current dashboard can be available publicly">Enabled</Label>
+                  <Switch
+                    disabled={dashboardHasTemplateVariables(dashboardVariables)}
+                    data-testid={selectors.EnableSwitch}
+                    value={publicDashboard?.isEnabled}
+                    onChange={() => {
+                      reportInteraction('grafana_dashboards_public_enable_clicked', {
+                        action: publicDashboard?.isEnabled ? 'disable' : 'enable',
+                      });

-                    setPublicDashboardConfig({
-                      ...publicDashboard,
-                      isEnabled: !publicDashboard.isEnabled,
-                    });
-                  }}
-                />
-              </Field>
-            </FieldSet>
-
-            <FieldSet>
-              {publicDashboardPersisted(publicDashboard) && publicDashboard.isEnabled && (
-                <Field label="Link URL">
-                  <Input
-                    value={generatePublicDashboardUrl(publicDashboard)}
-                    readOnly
-                    data-testid={selectors.CopyUrlInput}
-                    addonAfter={
-                      <ClipboardButton
-                        data-testid={selectors.CopyUrlButton}
-                        variant="primary"
-                        icon="copy"
-                        getText={() => {
-                          return generatePublicDashboardUrl(publicDashboard);
-                        }}
-                      >
-                        Copy
-                      </ClipboardButton>
-                    }
+                      setPublicDashboardConfig({
+                        ...publicDashboard,
+                        isEnabled: !publicDashboard.isEnabled,
+                      });
+                    }}
                  />
-                </Field>
-              )}
+                </HorizontalGroup>
+                {publicDashboardPersisted(publicDashboard) && publicDashboard.isEnabled && (
+                  <Field label="Link URL" className={styles.publicUrl}>
+                    <Input
+                      value={generatePublicDashboardUrl(publicDashboard)}
+                      readOnly
+                      data-testid={selectors.CopyUrlInput}
+                      addonAfter={
+                        <ClipboardButton
+                          data-testid={selectors.CopyUrlButton}
+                          variant="primary"
+                          icon="copy"
+                          getText={() => generatePublicDashboardUrl(publicDashboard)}
+                        >
+                          Copy
+                        </ClipboardButton>
+                      }
+                    />
+                  </Field>
+                )}
+              </VerticalGroup>
            </FieldSet>
-
            {hasWritePermissions ? (
              props.dashboard.hasUnsavedChanges() && (
                <Alert
@ -253,11 +247,11 @@ export const SharePublicDashboard = (props: Props) => {
              <Alert title="You don't have permissions to create or update a public dashboard" severity="warning" />
            )}
            <Button
-              disabled={!hasWritePermissions || !acknowledged() || props.dashboard.hasUnsavedChanges()}
+              disabled={!hasWritePermissions || !acknowledged || props.dashboard.hasUnsavedChanges()}
              onClick={onSavePublicConfig}
              data-testid={selectors.SaveConfigButton}
            >
-              Save Sharing Configuration
+              Save sharing configuration
            </Button>
          </div>
        </>
@ -265,3 +259,20 @@ export const SharePublicDashboard = (props: Props) => {
    </>
  );
 };
+
+const getStyles = (theme: GrafanaTheme2) => ({
+  checkboxes: css`
+    margin: ${theme.spacing(2, 0)};
+  `,
+  timeRange: css`
+    padding: ${theme.spacing(1, 1)};
+    margin: ${theme.spacing(0, 0, 2, 0)};
+  `,
+  dashboardConfig: css`
+    margin: ${theme.spacing(0, 0, 3, 0)};
+  `,
+  publicUrl: css`
+    width: 100%;
+    margin-bottom: 0;
+  `,
+});
--- a/public/app/features/dashboard/services/TimeSrv.ts
+++ b/public/app/features/dashboard/services/TimeSrv.ts
@ -15,6 +15,7 @@ import appEvents from 'app/core/app_events';
 import { config } from 'app/core/config';
 import { contextSrv, ContextSrv } from 'app/core/services/context_srv';
 import { getShiftedTimeRange, getZoomedTimeRange } from 'app/core/utils/timePicker';
+import { getTimeRange } from 'app/features/dashboard/utils/timeRange';

 import { AbsoluteTimeEvent, ShiftTimeEvent, ShiftTimeEventDirection, ZoomOutEvent } from '../../../types/events';
 import { TimeModel } from '../state/TimeModel';
@ -316,19 +317,7 @@ export class TimeSrv {
  };

  timeRange(): TimeRange {
-    // make copies if they are moment  (do not want to return out internal moment, because they are mutable!)
-    const raw = {
-      from: isDateTime(this.time.from) ? dateTime(this.time.from) : this.time.from,
-      to: isDateTime(this.time.to) ? dateTime(this.time.to) : this.time.to,
-    };
-
-    const timezone = this.timeModel ? this.timeModel.getTimezone() : undefined;
-
-    return {
-      from: dateMath.parse(raw.from, false, timezone, this.timeModel?.fiscalYearStartMonth)!,
-      to: dateMath.parse(raw.to, true, timezone, this.timeModel?.fiscalYearStartMonth)!,
-      raw: raw,
-    };
+    return getTimeRange(this.time, this.timeModel);
  }

  zoomOut(factor: number, updateUrl = true) {
--- a/public/app/features/dashboard/utils/timeRange.ts
+++ b/public/app/features/dashboard/utils/timeRange.ts
@ -0,0 +1,22 @@
+import { DateTime, TimeRange } from '@grafana/data';
+import { dateMath, dateTime, isDateTime } from '@grafana/data/src';
+import { TimeModel } from 'app/features/dashboard/state/TimeModel';
+
+export const getTimeRange = (
+  time: { from: DateTime | string; to: DateTime | string },
+  timeModel?: TimeModel
+): TimeRange => {
+  // make copies if they are moment  (do not want to return out internal moment, because they are mutable!)
+  const raw = {
+    from: isDateTime(time.from) ? dateTime(time.from) : time.from,
+    to: isDateTime(time.to) ? dateTime(time.to) : time.to,
+  };
+
+  const timezone = timeModel ? timeModel.getTimezone() : undefined;
+
+  return {
+    from: dateMath.parse(raw.from, false, timezone, timeModel?.fiscalYearStartMonth)!,
+    to: dateMath.parse(raw.to, true, timezone, timeModel?.fiscalYearStartMonth)!,
+    raw: raw,
+  };
+};