Role picker: Fix flickering at service accounts page (#77049)

* Role picker: Fix flickering at service accounts page * Set role picker fixed width * Fix betterer * Fix styles
2025-02-25 18:55:37 -06:00 · 2023-10-25 13:03:12 +02:00 · 2023-10-25 13:03:12 +02:00 · aa7a6da985
commit aa7a6da985
parent 1bc81b7bd1
10 changed files with 144 additions and 145 deletions
--- a/.betterer.results
+++ b/.betterer.results
@ -1395,27 +1395,6 @@ exports[`better eslint`] = {
    "public/app/core/components/RolePicker/ValueContainer.tsx:5381": [
      [0, 0, 0, "Styles should be written using objects.", "0"]
    ],
-    "public/app/core/components/RolePicker/styles.ts:5381": [
-      [0, 0, 0, "Styles should be written using objects.", "0"],
-      [0, 0, 0, "Styles should be written using objects.", "1"],
-      [0, 0, 0, "Styles should be written using objects.", "2"],
-      [0, 0, 0, "Styles should be written using objects.", "3"],
-      [0, 0, 0, "Styles should be written using objects.", "4"],
-      [0, 0, 0, "Styles should be written using objects.", "5"],
-      [0, 0, 0, "Styles should be written using objects.", "6"],
-      [0, 0, 0, "Styles should be written using objects.", "7"],
-      [0, 0, 0, "Styles should be written using objects.", "8"],
-      [0, 0, 0, "Styles should be written using objects.", "9"],
-      [0, 0, 0, "Styles should be written using objects.", "10"],
-      [0, 0, 0, "Styles should be written using objects.", "11"],
-      [0, 0, 0, "Styles should be written using objects.", "12"],
-      [0, 0, 0, "Styles should be written using objects.", "13"],
-      [0, 0, 0, "Styles should be written using objects.", "14"],
-      [0, 0, 0, "Styles should be written using objects.", "15"],
-      [0, 0, 0, "Styles should be written using objects.", "16"],
-      [0, 0, 0, "Styles should be written using objects.", "17"],
-      [0, 0, 0, "Styles should be written using objects.", "18"]
-    ],
    "public/app/core/components/Select/OldFolderPicker.tsx:5381": [
      [0, 0, 0, "Use data-testid for E2E selectors instead of aria-label", "0"],
      [0, 0, 0, "Styles should be written using objects.", "1"]
--- a/public/app/core/components/RolePicker/RolePicker.tsx
+++ b/public/app/core/components/RolePicker/RolePicker.tsx
@ -1,11 +1,12 @@
 import React, { FormEvent, useCallback, useEffect, useState, useRef } from 'react';

-import { ClickOutsideWrapper, HorizontalGroup, Spinner } from '@grafana/ui';
+import { ClickOutsideWrapper, Spinner, useStyles2, useTheme2 } from '@grafana/ui';
 import { Role, OrgRole } from 'app/types';

 import { RolePickerInput } from './RolePickerInput';
 import { RolePickerMenu } from './RolePickerMenu';
 import { MENU_MAX_HEIGHT, ROLE_PICKER_SUBMENU_MIN_WIDTH, ROLE_PICKER_WIDTH } from './constants';
+import { getStyles } from './styles';

 export interface Props {
  basicRole?: OrgRole;
@ -24,6 +25,7 @@ export interface Props {
   */
  apply?: boolean;
  maxWidth?: string | number;
+  width?: string | number;
 }

 export const RolePicker = ({
@ -40,6 +42,7 @@ export const RolePicker = ({
  canUpdateRoles = true,
  apply = false,
  maxWidth = ROLE_PICKER_WIDTH,
+  width,
 }: Props): JSX.Element | null => {
  const [isOpen, setOpen] = useState(false);
  const [selectedRoles, setSelectedRoles] = useState<Role[]>(appliedRoles);
@ -47,6 +50,9 @@ export const RolePicker = ({
  const [query, setQuery] = useState('');
  const [offset, setOffset] = useState({ vertical: 0, horizontal: 0 });
  const ref = useRef<HTMLDivElement>(null);
+  const styles = useStyles2(getStyles);
+  const theme = useTheme2();
+  const widthPx = typeof width === 'number' ? theme.spacing(width) : width;

  useEffect(() => {
    setSelectedBuiltInRole(basicRole);
@ -148,10 +154,10 @@ export const RolePicker = ({

  if (isLoading) {
    return (
-      <HorizontalGroup justify="center">
+      <div style={{ maxWidth: widthPx || maxWidth, width: widthPx }}>
        <span>Loading...</span>
-        <Spinner size={16} />
-      </HorizontalGroup>
+        <Spinner size={16} inline className={styles.loadingSpinner} />
+      </div>
    );
  }

@ -160,7 +166,8 @@ export const RolePicker = ({
      data-testid="role-picker"
      style={{
        position: 'relative',
-        maxWidth,
+        maxWidth: widthPx || maxWidth,
+        width: widthPx,
      }}
      ref={ref}
    >
@ -175,6 +182,7 @@ export const RolePicker = ({
          isFocused={isOpen}
          disabled={disabled}
          showBasicRole={showBasicRole}
+          width={widthPx}
        />
        {isOpen && (
          <RolePickerMenu
--- a/public/app/core/components/RolePicker/RolePickerInput.tsx
+++ b/public/app/core/components/RolePicker/RolePickerInput.tsx
@ -18,6 +18,7 @@ interface InputProps extends HTMLProps<HTMLInputElement> {
  showBasicRole?: boolean;
  isFocused?: boolean;
  disabled?: boolean;
+  width?: string;
  onQueryChange: (query?: string) => void;
  onOpen: (event: FormEvent<HTMLElement>) => void;
  onClose: () => void;
@ -30,12 +31,13 @@ export const RolePickerInput = ({
  isFocused,
  query,
  showBasicRole,
+  width,
  onOpen,
  onClose,
  onQueryChange,
  ...rest
 }: InputProps): JSX.Element => {
-  const styles = useStyles2(getRolePickerInputStyles, false, !!isFocused, !!disabled, false);
+  const styles = useStyles2(getRolePickerInputStyles, false, !!isFocused, !!disabled, false, width);
  const inputRef = useRef<HTMLInputElement | null>(null);

  useEffect(() => {
@ -125,7 +127,8 @@ const getRolePickerInputStyles = (
  invalid: boolean,
  focused: boolean,
  disabled: boolean,
-  withPrefix: boolean
+  withPrefix: boolean,
+  width?: string
 ) => {
  const styles = getInputStyles({ theme, invalid });

@ -139,7 +142,8 @@ const getRolePickerInputStyles = (
        `,
      disabled && styles.inputDisabled,
      css`
-        min-width: ${ROLE_PICKER_WIDTH}px;
+        min-width: ${width || ROLE_PICKER_WIDTH + 'px'};
+        width: ${width};
        min-height: 32px;
        height: auto;
        flex-direction: row;
--- a/public/app/core/components/RolePicker/TeamRolePicker.tsx
+++ b/public/app/core/components/RolePicker/TeamRolePicker.tsx
@ -27,6 +27,7 @@ export interface Props {
   */
  apply?: boolean;
  maxWidth?: string | number;
+  width?: string | number;
 }

 export const TeamRolePicker = ({
@ -37,6 +38,7 @@ export const TeamRolePicker = ({
  pendingRoles,
  apply = false,
  maxWidth,
+  width,
 }: Props) => {
  const [{ loading, value: appliedRoles = [] }, getTeamRoles] = useAsyncFn(async () => {
    try {
@ -81,6 +83,7 @@ export const TeamRolePicker = ({
      basicRoleDisabled={true}
      canUpdateRoles={canUpdateRoles}
      maxWidth={maxWidth}
+      width={width}
    />
  );
 };
--- a/public/app/core/components/RolePicker/UserRolePicker.tsx
+++ b/public/app/core/components/RolePicker/UserRolePicker.tsx
@ -31,6 +31,7 @@ export interface Props {
  onApplyRoles?: (newRoles: Role[], userId: number, orgId: number | undefined) => void;
  pendingRoles?: Role[];
  maxWidth?: string | number;
+  width?: string | number;
 }

 export const UserRolePicker = ({
@ -46,6 +47,7 @@ export const UserRolePicker = ({
  onApplyRoles,
  pendingRoles,
  maxWidth,
+  width,
 }: Props) => {
  const [{ loading, value: appliedRoles = [] }, getUserRoles] = useAsyncFn(async () => {
    try {
@ -98,6 +100,7 @@ export const UserRolePicker = ({
      apply={apply}
      canUpdateRoles={canUpdateRoles}
      maxWidth={maxWidth}
+      width={width}
    />
  );
 };
--- a/public/app/core/components/RolePicker/styles.ts
+++ b/public/app/core/components/RolePicker/styles.ts
@ -4,119 +4,119 @@ import { GrafanaTheme2 } from '@grafana/data';

 import { ROLE_PICKER_SUBMENU_MIN_WIDTH } from './constants';

-export const getStyles = (theme: GrafanaTheme2) => {
-  return {
-    hideScrollBar: css`
-      .scrollbar-view {
-        /* Hide scrollbar for Chrome, Safari, and Opera */
-        &::-webkit-scrollbar {
-          display: none;
-        }
-        /* Hide scrollbar for Firefox */
-        scrollbar-width: none;
-      }
-    `,
-    menuWrapper: css`
-      display: flex;
-      max-height: 650px;
-      position: absolute;
-      z-index: ${theme.zIndex.dropdown};
-      overflow: hidden;
-      min-width: auto;
-    `,
-    menu: css`
-      min-width: ${ROLE_PICKER_SUBMENU_MIN_WIDTH}px;
+export const getStyles = (theme: GrafanaTheme2) => ({
+  hideScrollBar: css({
+    '.scrollbar-view': {
+      /* Hide scrollbar for Chrome, Safari, and Opera */
+      '&::-webkit-scrollbar': {
+        display: 'none',
+      },
+      /* Hide scrollbar for Firefox */
+      scrollbarWidth: 'none',
+    },
+  }),
+  menuWrapper: css({
+    display: 'flex',
+    maxHeight: '650px',
+    position: 'absolute',
+    zIndex: theme.zIndex.dropdown,
+    overflow: 'hidden',
+    minWidth: 'auto',
+  }),
+  menu: css({
+    minWidth: `${ROLE_PICKER_SUBMENU_MIN_WIDTH}px`,
+    '& > div': {
+      paddingTop: theme.spacing(1),
+    },
+  }),
+  menuLeft: css({
+    right: 0,
+    flexDirection: 'row-reverse',
+  }),
+  subMenu: css({
+    height: '100%',
+    minWidth: `${ROLE_PICKER_SUBMENU_MIN_WIDTH}px`,
+    display: 'flex',
+    flexDirection: 'column',
+    borderLeft: `1px solid ${theme.components.input.borderColor}`,

-      & > div {
-        padding-top: ${theme.spacing(1)};
-      }
-    `,
-    menuLeft: css`
-      right: 0;
-      flex-direction: row-reverse;
-    `,
-    subMenu: css`
-      height: 100%;
-      min-width: ${ROLE_PICKER_SUBMENU_MIN_WIDTH}px;
-      display: flex;
-      flex-direction: column;
-      border-left: 1px solid ${theme.components.input.borderColor};
+    '& > div': {
+      paddingTop: theme.spacing(1),
+    },
+  }),
+  subMenuLeft: css({
+    borderRight: `1px solid ${theme.components.input.borderColor}`,
+    borderLeft: 'unset',
+  }),
+  groupHeader: css({
+    padding: theme.spacing(0, 4.5),
+    display: 'flex',
+    alignItems: 'center',
+    color: theme.colors.text.primary,
+    fontWeight: theme.typography.fontWeightBold,
+  }),
+  container: css({
+    padding: theme.spacing(1),
+    border: `1px ${theme.colors.border.weak} solid`,
+    borderRadius: theme.shape.radius.default,
+    backgroundColor: theme.colors.background.primary,
+    zIndex: theme.zIndex.modal,
+  }),
+  menuSection: css({
+    marginBottom: theme.spacing(2),
+  }),
+  menuOptionCheckbox: css({
+    display: 'flex',
+    margin: theme.spacing(0, 1, 0, 0.25),
+  }),
+  menuButtonRow: css({
+    backgroundColor: theme.colors.background.primary,
+    padding: theme.spacing(1),
+  }),
+  menuOptionBody: css({
+    fontWeight: theme.typography.fontWeightRegular,
+    padding: theme.spacing(0, 1.5, 0, 0),
+  }),
+  menuOptionDisabled: css({
+    color: theme.colors.text.disabled,
+    cursor: 'not-allowed',
+  }),
+  menuOptionExpand: css({
+    position: 'absolute',
+    right: theme.spacing(1.25),
+    color: theme.colors.text.disabled,

-      & > div {
-        padding-top: ${theme.spacing(1)};
-      }
-    `,
-    subMenuLeft: css`
-      border-right: 1px solid ${theme.components.input.borderColor};
-      border-left: unset;
-    `,
-    groupHeader: css`
-      padding: ${theme.spacing(0, 4.5)};
-      display: flex;
-      align-items: center;
-      color: ${theme.colors.text.primary};
-      font-weight: ${theme.typography.fontWeightBold};
-    `,
-    container: css`
-      padding: ${theme.spacing(1)};
-      border: 1px ${theme.colors.border.weak} solid;
-      border-radius: ${theme.shape.radius.default};
-      background-color: ${theme.colors.background.primary};
-      z-index: ${theme.zIndex.modal};
-    `,
-    menuSection: css`
-      margin-bottom: ${theme.spacing(2)};
-    `,
-    menuOptionCheckbox: css`
-      display: flex;
-      margin: ${theme.spacing(0, 1, 0, 0.25)};
-    `,
-    menuButtonRow: css`
-      background-color: ${theme.colors.background.primary};
-      padding: ${theme.spacing(1)};
-    `,
-    menuOptionBody: css`
-      font-weight: ${theme.typography.fontWeightRegular};
-      padding: ${theme.spacing(0, 1.5, 0, 0)};
-    `,
-    menuOptionDisabled: css`
-      color: ${theme.colors.text.disabled};
-      cursor: not-allowed;
-    `,
-    menuOptionExpand: css`
-      position: absolute;
-      right: ${theme.spacing(1.25)};
-      color: ${theme.colors.text.disabled};
-
-      &:after {
-        content: '>';
-      }
-    `,
-    menuOptionInfoSign: css`
-      color: ${theme.colors.text.disabled};
-    `,
-    basicRoleSelector: css`
-      margin: ${theme.spacing(1, 1.25, 1, 1.5)};
-    `,
-    subMenuPortal: css`
-      height: 100%;
-      > div {
-        height: 100%;
-      }
-    `,
-    subMenuButtonRow: css`
-      background-color: ${theme.colors.background.primary};
-      padding: ${theme.spacing(1)};
-    `,
-    checkboxPartiallyChecked: css`
-      input {
-        &:checked + span {
-          &:after {
-            border-width: 0 3px 0px 0;
-            transform: rotate(90deg);
-          }
-        }
-      }
-    `,
-  };
-};
+    '&:after': {
+      content: '">"',
+    },
+  }),
+  menuOptionInfoSign: css({
+    color: theme.colors.text.disabled,
+  }),
+  basicRoleSelector: css({
+    margin: theme.spacing(1, 1.25, 1, 1.5),
+  }),
+  subMenuPortal: css({
+    height: '100%',
+    '> div': {
+      height: '100%',
+    },
+  }),
+  subMenuButtonRow: css({
+    backgroundColor: theme.colors.background.primary,
+    padding: theme.spacing(1),
+  }),
+  checkboxPartiallyChecked: css({
+    input: {
+      '&:checked + span': {
+        '&:after': {
+          borderWidth: '0 3px 0px 0',
+          transform: 'rotate(90deg)',
+        },
+      },
+    },
+  }),
+  loadingSpinner: css({
+    marginLeft: theme.spacing(1),
+  }),
+});
--- a/public/app/features/admin/Users/OrgUsersTable.tsx
+++ b/public/app/features/admin/Users/OrgUsersTable.tsx
@ -132,6 +132,7 @@ export const OrgUsersTable = ({
              onBasicRoleChange={(newRole) => onRoleChange(newRole, original)}
              basicRoleDisabled={basicRoleDisabled}
              basicRoleDisabledMessage={disabledRoleMessage}
+              width={40}
            />
          ) : (
            <OrgRolePicker
--- a/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx
+++ b/public/app/features/serviceaccounts/ServiceAccountsListPage.tsx
@ -234,7 +234,7 @@ export const ServiceAccountsListPageUnconnected = ({
                    <th>ID</th>
                    <th>Roles</th>
                    <th>Tokens</th>
-                    <th style={{ width: '34px' }} />
+                    <th style={{ width: '120px' }} />
                  </tr>
                </thead>
                <tbody>
--- a/public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx
+++ b/public/app/features/serviceaccounts/components/ServiceAccountsListItem.tsx
@ -81,6 +81,7 @@ const ServiceAccountListItem = memo(
                roleOptions={roleOptions}
                basicRoleDisabled={!canUpdateRole}
                disabled={serviceAccount.isDisabled}
+                width={40}
              />
            )}
          </td>
--- a/public/app/features/teams/TeamList.tsx
+++ b/public/app/features/teams/TeamList.tsx
@ -96,7 +96,7 @@ export const TeamList = ({
                  AccessControlAction.ActionTeamsRolesList,
                  original
                );
-                return canSeeTeamRoles && <TeamRolePicker teamId={original.id} roleOptions={roleOptions} />;
+                return canSeeTeamRoles && <TeamRolePicker teamId={original.id} roleOptions={roleOptions} width={40} />;
              },
            },
          ]