Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [M3-7490] - Improve NodeBalancer Restricted User Experience #10095

Merged
merged 16 commits into from
Jan 31, 2024
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Tech Stories
---

Improve NodeBalancer Restricted User Experience ([#10095](https://github.com/linode/manager/pull/10095))
14 changes: 8 additions & 6 deletions packages/manager/src/components/ActionMenu/ActionMenu.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { IconButton, ListItemText } from '@mui/material';
import { IconButton, ListItemText, useTheme } from '@mui/material';
import Menu from '@mui/material/Menu';
import MenuItem from '@mui/material/MenuItem';
import * as React from 'react';
Expand Down Expand Up @@ -37,6 +37,7 @@ export interface ActionMenuProps {
*/
export const ActionMenu = React.memo((props: ActionMenuProps) => {
const { actionsList, ariaLabel, onOpen } = props;
const theme = useTheme();

const menuId = convertToKebabCase(ariaLabel);
const buttonId = `${convertToKebabCase(ariaLabel)}-button`;
Expand Down Expand Up @@ -69,14 +70,15 @@ export const ActionMenu = React.memo((props: ActionMenuProps) => {
}

const sxTooltipIcon = {
'& .MuiSvgIcon-root': {
fill: '#fff',
height: '20px',
width: '20px',
},
'& :hover': {
color: '#4d99f1',
},
'&& .MuiSvgIcon-root': {
fill: theme.color.disabledText,
height: '20px',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only display the tooltip for when things are disabled

width: '20px',
},

color: '#fff',
padding: '0 0 0 8px',
pointerEvents: 'all', // Allows the tooltip to be hovered on a disabled MenuItem
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { action } from '@storybook/addon-actions';
import React from 'react';

import { StyledPlusIcon, StyledTagButton } from './StyledTagButton';

import type { Meta, StoryObj } from '@storybook/react';

const meta: Meta<typeof StyledTagButton> = {
args: {
children: 'Tag',
disabled: false,
onClick: () => null,
},
component: StyledTagButton,
title: 'Components/TagButton',
};

export default meta;

type Story = StoryObj<typeof StyledTagButton>;

export const Default: Story = {
args: {
buttonType: 'outlined',
children: 'Tag',
disabled: false,
onClick: action('onClick'),
},
render: (args) => (
<StyledTagButton
{...args}
endIcon={<StyledPlusIcon disabled={args.disabled} />}
>
Add a Tag
</StyledTagButton>
),
};
38 changes: 38 additions & 0 deletions packages/manager/src/components/Button/StyledTagButton.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { styled } from '@mui/material/styles';

import Plus from 'src/assets/icons/plusSign.svg';

import { Button } from './Button';

/**
* A button for Tags. Eventually this treatment will go away,
* but the sake of the MUI migration we need to keep it around for now, and as a styled component in order to get rid of
* spreading excessive styles for everywhere this is used.
*
*/
export const StyledTagButton = styled(Button, {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use this in other places as well, but I didn't want to expand the scope of this PR too much.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ‘ that being said we should both add a unit test and add/modify a story if it is meant to stay

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with this new component and thanks for creating a test/story as per @abailly-akamai. Should we open a ticket for the other places?

label: 'StyledTagButton',
})(({ theme, ...props }) => ({
border: 'none',
fontSize: '0.875rem',
...(!props.disabled && {
'&:hover, &:focus': {
backgroundColor: theme.color.tagButton,
border: 'none',
},
backgroundColor: theme.color.tagButton,
color: theme.textColors.linkActiveLight,
}),
}));

export const StyledPlusIcon = styled(Plus, {
label: 'StyledPlusIcon',
})(({ theme, ...props }) => ({
color: props.disabled
? theme.name === 'dark'
? '#5c6470'
: theme.color.disabledText
: theme.color.tagIcon,
height: '10px',
width: '10px',
}));
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { Autocomplete } from '../Autocomplete/Autocomplete';
import { LinkButton } from '../LinkButton';

interface Props {
disabled?: boolean;
entityType: FirewallDeviceEntityType | undefined;
handleFirewallChange: (firewallID: number) => void;
helperText: JSX.Element;
Expand All @@ -21,6 +22,7 @@ interface Props {

export const SelectFirewallPanel = (props: Props) => {
const {
disabled,
entityType,
handleFirewallChange,
helperText,
Expand Down Expand Up @@ -69,6 +71,7 @@ export const SelectFirewallPanel = (props: Props) => {
onChange={(_, selection) => {
handleFirewallChange(selection?.value ?? -1);
}}
disabled={disabled}
errorText={error?.[0].reason}
label="Assign Firewall"
loading={isLoading}
Expand All @@ -78,7 +81,7 @@ export const SelectFirewallPanel = (props: Props) => {
value={selectedFirewall}
/>
<StyledLinkButtonBox>
<LinkButton onClick={handleCreateFirewallClick}>
<LinkButton isDisabled={disabled} onClick={handleCreateFirewallClick}>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To disable this link in create flows for restricted users, probably should've been passed in to begin with.

Create Firewall
</LinkButton>
</StyledLinkButtonBox>
Expand Down
31 changes: 5 additions & 26 deletions packages/manager/src/components/TagCell/TagCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import { styled } from '@mui/material/styles';
import { SxProps } from '@mui/system';
import * as React from 'react';

import Plus from 'src/assets/icons/plusSign.svg';
import { CircleProgress } from 'src/components/CircleProgress';
import { IconButton } from 'src/components/IconButton';
import { Tag } from 'src/components/Tag/Tag';
import { omittedProps } from 'src/utilities/omittedProps';

import { StyledPlusIcon, StyledTagButton } from '../Button/StyledTagButton';
import { AddTag } from './AddTag';

interface TagCellProps {
Expand Down Expand Up @@ -108,13 +108,14 @@ const TagCell = (props: TagCellProps) => {
<MoreHoriz />
</StyledIconButton>
) : null}
<StyledAddTagButton
<StyledTagButton
buttonType="outlined"
endIcon={<StyledPlusIcon />}
onClick={() => setAddingTag(true)}
title="Add a tag"
>
Add a tag
<Plus />
</StyledAddTagButton>
</StyledTagButton>
</>
)}
</StyledGrid>
Expand Down Expand Up @@ -184,25 +185,3 @@ const StyledIconButton = styled(IconButton)(({ theme }) => ({
},
width: '40px',
}));

const StyledAddTagButton = styled('button')(({ theme }) => ({
'& svg': {
color: theme.color.tagIcon,
height: 10,
marginLeft: 10,
width: 10,
},
alignItems: 'center',
backgroundColor: theme.color.tagButton,
border: 'none',
borderRadius: 3,
color: theme.textColors.linkActiveLight,
cursor: 'pointer',
display: 'flex',
fontFamily: theme.font.bold,
fontSize: 14,
height: 30,
paddingLeft: 10,
paddingRight: 10,
whiteSpace: 'nowrap',
}));
20 changes: 0 additions & 20 deletions packages/manager/src/components/TagsPanel/TagsPanel.styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,6 @@ export const useStyles = makeStyles()((theme: Theme) => ({
justifyContent: 'flex-start',
width: '100%',
},
addTagButton: {
'& svg': {
color: theme.color.tagIcon,
height: 10,
marginLeft: 10,
width: 10,
},
alignItems: 'center',
backgroundColor: theme.color.tagButton,
border: 'none',
borderRadius: 3,
color: theme.textColors.linkActiveLight,
cursor: 'pointer',
display: 'flex',
fontFamily: theme.font.bold,
fontSize: '0.875rem',
justifyContent: 'center',
padding: '7px 10px',
whiteSpace: 'nowrap',
},
errorNotice: {
'& .noticeText': {
fontFamily: '"LatoWeb", sans-serif',
Expand Down
16 changes: 10 additions & 6 deletions packages/manager/src/components/TagsPanel/TagsPanel.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import * as React from 'react';
import { useQueryClient } from 'react-query';

import Plus from 'src/assets/icons/plusSign.svg';
import {
StyledPlusIcon,
StyledTagButton,
} from 'src/components/Button/StyledTagButton';
import { CircleProgress } from 'src/components/CircleProgress';
import Select from 'src/components/EnhancedSelect/Select';
import { Tag } from 'src/components/Tag/Tag';
Expand Down Expand Up @@ -162,6 +165,7 @@ export const TagsPanel = (props: TagsPanelProps) => {
className={classes.selectTag}
creatable
createOptionPosition="first"
disabled={disabled}
escapeClearsValue
hideLabel
isLoading={userTagsLoading}
Expand All @@ -178,14 +182,14 @@ export const TagsPanel = (props: TagsPanelProps) => {
[classes.hasError]: tagError.length > 0,
})}
>
<button
className={classes.addTagButton}
<StyledTagButton
buttonType="outlined"
disabled={disabled}
endIcon={<StyledPlusIcon disabled={disabled} />}
onClick={toggleTagInput}
title="Add a tag"
>
Add a tag
<Plus />
</button>
</StyledTagButton>
</div>
)}
<div className={classes.tagsPanelItemWrapper}>
Expand Down
13 changes: 13 additions & 0 deletions packages/manager/src/features/Account/constants.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,19 @@
export const BUSINESS_PARTNER = 'business partner';
export const ADMINISTRATOR = 'administrator';

export const grantTypeMap = {
abailly-akamai marked this conversation as resolved.
Show resolved Hide resolved
database: 'Databases',
domain: 'Domains',
firewall: 'Firewalls',
image: 'Images',
linode: 'Linodes',
longview: 'Longview Clients',
nodebalancer: 'NodeBalancers',
stackscript: 'StackScripts',
volume: 'Volumes',
vpc: 'VPCs',
} as const;

export const PARENT_PROXY_USER_CLOSE_ACCOUNT_TOOLTIP_TEXT =
'Remove indirect customers before closing the account.';
export const CHILD_USER_CLOSE_ACCOUNT_TOOLTIP_TEXT =
Expand Down
6 changes: 6 additions & 0 deletions packages/manager/src/features/Account/types.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { grantTypeMap } from 'src/features/Account/constants';

// A useful type for getting the values of an object
export type ObjectValues<T> = T[keyof T];

export type GrantTypeMap = ObjectValues<typeof grantTypeMap>;
abailly-akamai marked this conversation as resolved.
Show resolved Hide resolved
34 changes: 33 additions & 1 deletion packages/manager/src/features/Account/utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,38 @@
import { getStorage, setStorage } from 'src/utilities/storage';

import type { Token } from '@linode/api-v4';
import type { GlobalGrantTypes, Grants, Profile, Token } from '@linode/api-v4';
import type { GrantTypeMap } from 'src/features/Account/types';

type ActionType = 'create' | 'delete' | 'edit' | 'view';

interface GetRestrictedResourceText {
action?: ActionType;
isSingular?: boolean;
resourceType: GrantTypeMap;
}
export const getRestrictedResourceText = ({
action = 'edit',
isSingular = true,
resourceType,
}: GetRestrictedResourceText): string => {
const resource = isSingular
? 'this ' + resourceType.replace(/s$/, '')
: resourceType;

return `You don't have permissions to ${action} ${resource}. Please contact your account administrator to request the necessary permissions.`;
};

export const isRestrictedGlobalGrantType = ({
globalGrantType,
grants,
profile,
}: {
globalGrantType: GlobalGrantTypes;
grants: Grants | undefined;
profile: Profile | undefined;
}): boolean => {
return Boolean(profile?.restricted) && !grants?.global[globalGrantType];
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it's clear from the name however

  1. can we add return types here?
  2. i think it's always super useful to complement those utils with JSDocs


// TODO: Parent/Child: FOR MSW ONLY, REMOVE WHEN API IS READY
// ================================================================
Expand Down
Loading
Loading