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

upcoming: [DI-20709] - CSS updates for widget level filters for ACLP #10903

Merged
merged 23 commits into from
Sep 17, 2024
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
41b1a3d
upcoming: [DI-20709] - CSS updates for widget level filters
venkat199501 Sep 9, 2024
1deb9ee
Merge branch 'develop' of https://github.com/linode/manager into feat…
venkat199501 Sep 9, 2024
d4b2528
upcoming: [DI-20709] - ES lint fixes
venkat199501 Sep 9, 2024
b0abd06
upcoming: [DI-20709] - Added comments
venkat199501 Sep 9, 2024
e0e7f16
upcoming: [DI-20709] - Added changeset
venkat199501 Sep 9, 2024
cb8dc53
upcoming: [DI-20709] - Color updates for zoomer component with use th…
venkat199501 Sep 9, 2024
0764a82
upcoming: [DI-20585] - CSS changes
venkat199501 Sep 10, 2024
00d005d
upcoming: [DI-20585] - Removed unused values and imports
venkat199501 Sep 10, 2024
4611a69
upcoming: [DI-20585] - Removed unused values and imports
venkat199501 Sep 10, 2024
68a4610
upcoming: [DI-20585] - Use common utils
venkat199501 Sep 10, 2024
f00d7a7
upcoming: [DI-20585] - Comment updates
venkat199501 Sep 10, 2024
add192c
upcoming: [DI-20585] - Remove any
venkat199501 Sep 10, 2024
17d8454
upcoming: [DI-20585] - Removing height and adjusting width. Removed d…
venkat199501 Sep 11, 2024
8cedef8
upcoming: [DI-20585] - Removed placeholder on selection and UT updates
venkat199501 Sep 11, 2024
f25ec83
upcoming: [DI-20585] - UT updates
venkat199501 Sep 11, 2024
c9bc84f
upcoming: [DI-20585] - Using form control for width of filters
venkat199501 Sep 12, 2024
2a20ac8
upcoming: [DI-20585] - Alignment fix
venkat199501 Sep 12, 2024
d038ea3
upcoming: [DI-20585] - PR comments
venkat199501 Sep 13, 2024
b3e5bc3
upcoming: [DI-20585] - PR comments
venkat199501 Sep 13, 2024
c319787
Merge branch 'develop' of https://github.com/linode/manager into feat…
venkat199501 Sep 14, 2024
2b44bbf
upcoming: [DI-20585] - Updated code syntax for handling small size sc…
venkat199501 Sep 15, 2024
eb7fe32
upcoming: [DI-20585] - CamelCase for properties
venkat199501 Sep 15, 2024
2383332
upcoming: [DI-20585] - Style to sx
venkat199501 Sep 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@linode/manager": Upcoming Features
---

Update CSS for widget level filters and widget heading title for ACLP ([#10903](https://github.com/linode/manager/pull/10903))
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { getUserPreferenceObject } from '../Utils/UserPreference';
import { createObjectCopy } from '../Utils/utils';
import { CloudPulseWidget } from '../Widget/CloudPulseWidget';
import {
all_interval_options,
allIntervalOptions,
getInSeconds,
getIntervalIndex,
} from '../Widget/components/CloudPulseIntervalSelect';
Expand Down Expand Up @@ -124,7 +124,7 @@ export const CloudPulseDashboard = (props: DashboardProperties) => {
const getTimeGranularity = (scrapeInterval: string) => {
const scrapeIntervalValue = getInSeconds(scrapeInterval);
const index = getIntervalIndex(scrapeIntervalValue);
return index < 0 ? all_interval_options[0] : all_interval_options[index];
return index < 0 ? allIntervalOptions[0] : allIntervalOptions[index];
};

const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const queryMocks = vi.hoisted(() => ({
const selectTimeDurationPlaceholder = 'Select Time Duration';
const circleProgress = 'circle-progress';
const mandatoryFiltersError = 'Mandatory Filters not Selected';
const customNodeTypePlaceholder = 'Select Node Type';

vi.mock('src/queries/cloudpulse/dashboards', async () => {
const actual = await vi.importActual('src/queries/cloudpulse/dashboards');
Expand Down Expand Up @@ -98,8 +97,7 @@ describe('CloudPulseDashboardWithFilters component tests', () => {

expect(screen.getByTestId('CloseIcon')).toBeDefined();

const inputBox = screen.getByPlaceholderText(customNodeTypePlaceholder);
fireEvent.change(inputBox, { target: { value: '' } }); // clear the value
fireEvent.click(screen.getByTitle('Clear')); // clear the value
expect(screen.getByText(mandatoryFiltersError)).toBeDefined();
});

Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { styled } from '@mui/material';

import { Autocomplete } from 'src/components/Autocomplete/Autocomplete';
import { isToday } from 'src/utilities/isToday';
import { getMetrics } from 'src/utilities/statMetrics';

Expand Down Expand Up @@ -331,3 +334,18 @@ export const isDataEmpty = (data: DataSet[]): boolean => {
thisSeries.data.every((thisPoint) => thisPoint[1] === null)
);
};

/**
* Returns an autocomplete with updated styles according to UX, this will be used at widget level
*/
export const StyledWidgetAutocomplete = styled(Autocomplete, {
label: 'StyledAutocomplete',
})(() => ({
'&& .MuiFormControl-root': {
'@media (max-width: 600px)': {
Copy link
Contributor

Choose a reason for hiding this comment

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

Using our theme.breakpoint values is a better way to do this. For example:

export const StyledWidgetAutocomplete = styled(Autocomplete, {
  label: 'StyledAutocomplete',
})(({ theme }) => ({
  '&& .MuiFormControl-root': {
    minWidth: '90px',
    [theme.breakpoints.down('sm')]: {
      width: '100%', // 100% width for xs and small screens
    },
    width: '90px',
  },
}));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

width: '100%', // 100% width for xs and small screens (max-width: 600px)
},
minWidth: '90px',
width: '90px',
},
}));
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { Box, Grid, Paper, Stack, Typography } from '@mui/material';
import { DateTime } from 'luxon';
import React from 'react';

import { Divider } from 'src/components/Divider';
import { useFlags } from 'src/hooks/useFlags';
import { useCloudPulseMetricsQuery } from 'src/queries/cloudpulse/metrics';
import { useProfile } from 'src/queries/profile/profile';
Expand Down Expand Up @@ -298,19 +297,15 @@ export const CloudPulseWidget = (props: CloudPulseWidgetProperties) => {
justifyContent={{ sm: 'space-between' }}
padding={1}
>
<Typography
fontSize={{ sm: '1.5rem', xs: '2rem' }}
marginLeft={1}
variant="h1"
>
<Typography marginLeft={1} variant="h2">
{convertStringToCamelCasesWithSpaces(widget.label)}{' '}
{!isLoading &&
`(${currentUnit}${unit.endsWith('ps') ? '/s' : ''})`}
</Typography>
<Stack
alignItems={'center'}
direction={{ sm: 'row' }}
gap={1}
gap={{ md: 2, xs: 1 }}
width={{ sm: 'inherit', xs: '100%' }}
>
{availableMetrics?.scrape_interval && (
Expand Down Expand Up @@ -339,7 +334,6 @@ export const CloudPulseWidget = (props: CloudPulseWidgetProperties) => {
</Box>
</Stack>
</Stack>
<Divider />

<CloudPulseLineGraph
error={
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';

import { Autocomplete } from 'src/components/Autocomplete/Autocomplete';
import { StyledWidgetAutocomplete } from '../../Utils/CloudPulseWidgetUtils';

export interface AggregateFunctionProperties {
/**
Expand Down Expand Up @@ -37,7 +37,7 @@ export const CloudPulseAggregateFunction = React.memo(
) || props.availableAggregateFunctions[0];

return (
<Autocomplete
<StyledWidgetAutocomplete
isOptionEqualToValue={(option, value) => {
return option.label == value.label;
}}
Expand Down
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason why we're not placing the options side by side and stacking them instead? There's a lot of empty space, so it looks a bit odd.

Screenshot 2024-09-12 at 1 35 44β€―PM

Copy link
Contributor

Choose a reason for hiding this comment

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

They used to be longer, but with the changes this looks weird. You're correct, the grid will need to be adjusted now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

@mjac0bs / @jaalah-akamai , for smaller screens we have made the autocomplete to occupy 100 percent width now, please check

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @venkymano-akamai, this looks much better on small screens. I left one comment about how to write that styling rule better to rely on our theme.

Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
import React from 'react';

import { Autocomplete } from 'src/components/Autocomplete/Autocomplete';
import { StyledWidgetAutocomplete } from '../../Utils/CloudPulseWidgetUtils';

import type { TimeGranularity } from '@linode/api-v4';

interface IntervalOptions {
label: string;
unit: string;
value: number;
}

export interface IntervalSelectProperties {
/**
* Default time granularity to be selected
Expand Down Expand Up @@ -39,7 +45,7 @@ export const getInSeconds = (interval: string) => {
};

// Intervals must be in ascending order here
export const all_interval_options = [
export const allIntervalOptions: IntervalOptions[] = [
{
label: '1 min',
unit: 'min',
Expand All @@ -62,14 +68,14 @@ export const all_interval_options = [
},
];

const autoIntervalOption = {
const autoIntervalOption: IntervalOptions = {
label: 'Auto',
unit: 'Auto',
value: -1,
};

export const getIntervalIndex = (scrapeIntervalValue: number) => {
return all_interval_options.findIndex(
return allIntervalOptions.findIndex(
(interval) =>
scrapeIntervalValue <=
getInSeconds(String(interval.value) + interval.unit.slice(0, 1))
Expand All @@ -83,18 +89,18 @@ export const CloudPulseIntervalSelect = React.memo(
const firstIntervalIndex = getIntervalIndex(scrapeIntervalValue);

// all intervals displayed if srape interval > highest available interval. Error handling done by api
const available_interval_options =
const availableIntervalOptions =
firstIntervalIndex < 0
? all_interval_options.slice()
: all_interval_options.slice(
? allIntervalOptions.slice()
: allIntervalOptions.slice(
firstIntervalIndex,
all_interval_options.length
allIntervalOptions.length
);

let default_interval =
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: default_interval existed before this PR, but same comment about preferring camel case over snake case to keep in mind when going forward.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

props.default_interval?.unit === 'Auto'
? autoIntervalOption
: available_interval_options.find(
: availableIntervalOptions.find(
(obj) =>
obj.value === props.default_interval?.value &&
obj.unit === props.default_interval?.unit
Expand All @@ -109,11 +115,17 @@ export const CloudPulseIntervalSelect = React.memo(
}

return (
<Autocomplete
isOptionEqualToValue={(option, value) => {
<StyledWidgetAutocomplete
isOptionEqualToValue={(
option: IntervalOptions,
value: IntervalOptions
) => {
return option?.value === value?.value && option?.unit === value?.unit;
}}
onChange={(_: any, selectedInterval: any) => {
onChange={(
_: React.SyntheticEvent,
selectedInterval: IntervalOptions
) => {
props.onIntervalChange({
unit: selectedInterval?.unit,
value: selectedInterval?.value,
Expand All @@ -127,7 +139,7 @@ export const CloudPulseIntervalSelect = React.memo(
fullWidth={false}
label="Select an Interval"
noMarginTop={true}
options={[autoIntervalOption, ...available_interval_options]}
options={[autoIntervalOption, ...availableIntervalOptions]}
sx={{ width: { xs: '100%' } }}
/>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import ZoomInMap from '@mui/icons-material/ZoomInMap';
import ZoomOutMap from '@mui/icons-material/ZoomOutMap';
import { useTheme } from '@mui/material/styles';
import * as React from 'react';

export interface ZoomIconProperties {
Expand All @@ -9,6 +10,8 @@ export interface ZoomIconProperties {
}

export const ZoomIcon = React.memo((props: ZoomIconProperties) => {
const theme = useTheme();

const handleClick = (needZoomIn: boolean) => {
props.handleZoomToggle(needZoomIn);
};
Expand All @@ -17,18 +20,26 @@ export const ZoomIcon = React.memo((props: ZoomIconProperties) => {
if (props.zoomIn) {
return (
<ZoomInMap
style={{
color: theme.color.grey1,
fontSize: 'x-large',
height: '34px',
}}
data-testid="zoom-in"
onClick={() => handleClick(false)}
style={{ color: 'grey', fontSize: 'x-large' }}
/>
);
}

return (
<ZoomOutMap
style={{
color: theme.color.grey1,
fontSize: 'x-large',
height: '34px',
}}
Copy link
Contributor

@mjac0bs mjac0bs Sep 13, 2024

Choose a reason for hiding this comment

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

Can we use the sx prop here? (docs reference)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

data-testid="zoom-out"
onClick={() => handleClick(true)}
style={{ color: 'grey', fontSize: 'x-large' }}
/>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,7 @@ describe('CloudPulseCustomSelect component tests', () => {
type={CloudPulseSelectTypes.static}
/>
);

expect(screen.getByPlaceholderText(testFilter)).toBeDefined();
expect(screen.queryByPlaceholderText(testFilter)).toBeNull();
const keyDown = screen.getByTestId(keyboardArrowDownIcon);
fireEvent.click(keyDown);
fireEvent.click(screen.getByText('Test1'));
Expand All @@ -82,8 +81,7 @@ describe('CloudPulseCustomSelect component tests', () => {
type={CloudPulseSelectTypes.static}
/>
);

expect(screen.getByPlaceholderText(testFilter)).toBeDefined();
expect(screen.queryByPlaceholderText(testFilter)).toBeNull();
const keyDown = screen.getByTestId(keyboardArrowDownIcon);
fireEvent.click(keyDown);
expect(screen.getAllByText('Test1').length).toEqual(2); // here it should be 2
Expand Down Expand Up @@ -111,13 +109,17 @@ describe('CloudPulseCustomSelect component tests', () => {
type={CloudPulseSelectTypes.dynamic}
/>
);
expect(screen.getByPlaceholderText(testFilter)).toBeDefined();
expect(screen.queryByPlaceholderText(testFilter)).toBeNull();
const keyDown = screen.getByTestId(keyboardArrowDownIcon);
fireEvent.click(keyDown);
fireEvent.click(screen.getByText('Test1'));
const textField = screen.getByTestId('textfield-input');
expect(textField.getAttribute('value')).toEqual('Test1');
expect(selectionChnage).toHaveBeenCalledTimes(1);

// if we click on clear icon , placeholder should appear for single select
fireEvent.click(screen.getByTitle('Clear'));
expect(screen.getByPlaceholderText(testFilter)).toBeDefined();
});

it('should render a component successfully with required props dynamic multi select', () => {
Expand All @@ -133,7 +135,7 @@ describe('CloudPulseCustomSelect component tests', () => {
type={CloudPulseSelectTypes.dynamic}
/>
);
expect(screen.getByPlaceholderText(testFilter)).toBeDefined();
expect(screen.queryByPlaceholderText(testFilter)).toBeNull();
const keyDown = screen.getByTestId(keyboardArrowDownIcon);
fireEvent.click(keyDown);
expect(screen.getAllByText('Test1').length).toEqual(2); // here it should be 2
Expand All @@ -148,5 +150,9 @@ describe('CloudPulseCustomSelect component tests', () => {
expect(screen.getAllByText('Test1').length).toEqual(1);
expect(screen.getAllByText('Test2').length).toEqual(1);
expect(selectionChnage).toHaveBeenCalledTimes(2); // check if selection change is called twice as we selected two options

// if we click on clear icon , placeholder should appear
fireEvent.click(screen.getByTitle('Clear'));
expect(screen.getByPlaceholderText(testFilter)).toBeDefined();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ export const CloudPulseCustomSelect = React.memo(
};

let staticErrorText = '';

// check for input prop errors
if (
(CloudPulseSelectTypes.static === type &&
Expand Down Expand Up @@ -205,6 +204,12 @@ export const CloudPulseCustomSelect = React.memo(
? options ?? []
: queriedResources ?? []
}
placeholder={
selectedResource &&
(!Array.isArray(selectedResource) || selectedResource.length)
? ''
: placeholder || 'Select a Value'
}
textFieldProps={{
hideLabel: true,
}}
Expand All @@ -214,7 +219,6 @@ export const CloudPulseCustomSelect = React.memo(
label="Select a Value"
multiple={isMultiSelect}
onChange={handleChange}
placeholder={placeholder ?? 'Select a Value'}
value={selectedResource ?? (isMultiSelect ? [] : null)}
/>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ export const CloudPulseResourcesSelect = React.memo(
setSelectedResources(resourceSelections);
handleResourcesSelection(resourceSelections);
}}
placeholder={
selectedResources?.length ? '' : placeholder || 'Select Resources'
}
textFieldProps={{
InputProps: {
sx: {
Expand All @@ -111,7 +114,6 @@ export const CloudPulseResourcesSelect = React.memo(
limitTags={2}
multiple
options={getResourcesList()}
placeholder={placeholder ? placeholder : 'Select Resources'}
value={selectedResources}
/>
);
Expand Down
Loading