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.

Sign up for GitHub

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

[CircularProgress] Custom variant #22174

Closed
wants to merge 6 commits into from
Closed

[CircularProgress] Custom variant #22174

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c9e880a
added custom variant
mnajdova Aug 13, 2020
337d407
proptypes + docs:api
mnajdova Aug 13, 2020
04015f7
framer
mnajdova Aug 13, 2020
706d357
framer hidden check
mnajdova Aug 13, 2020
f53829e
props as any
mnajdova Aug 14, 2020
d1c481b
Merge branch 'next' into feat/circular-progress-custom-variant
mnajdova Aug 14, 2020
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
2 changes: 1 addition & 1 deletion docs/pages/api-docs/circular-progress.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ The `MuiCircularProgress` name can be used for providing [default props](/custom
| <span class="prop-name">size</span> | <span class="prop-type">number<br>&#124;&nbsp;string</span> | <span class="prop-default">40</span> | The size of the circle. If using a number, the pixel unit is assumed. If using a string, you need to provide the CSS unit, e.g '3rem'. |
| <span class="prop-name">thickness</span> | <span class="prop-type">number</span> | <span class="prop-default">3.6</span> | The thickness of the circle. |
| <span class="prop-name">value</span> | <span class="prop-type">number</span> | <span class="prop-default">0</span> | The value of the progress indicator for the determinate variant. Value between 0 and 100. |
| <span class="prop-name">variant</span> | <span class="prop-type">'determinate'<br>&#124;&nbsp;'indeterminate'</span> | <span class="prop-default">'indeterminate'</span> | The variant to use. Use indeterminate when there is no progress value. |
| <span class="prop-name">variant</span> | <span class="prop-type">'determinate'<br>&#124;&nbsp;'indeterminate'<br>&#124;&nbsp;string</span> | <span class="prop-default">'indeterminate'</span> | The variant to use. Use indeterminate when there is no progress value. |

The `ref` is forwarded to the root element.

Expand Down
9 changes: 1 addition & 8 deletions framer/Material-UI.framerfx/code/CircularProgress.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ interface Props {
color: 'inherit' | 'primary' | 'secondary';
thickness: number;
value: number;
variant: 'determinate' | 'indeterminate';
width: number | string;
height: number;
}
Expand All @@ -21,7 +20,6 @@ CircularProgress.defaultProps = {
color: 'primary' as 'primary',
thickness: 4,
value: 75,
variant: 'determinate' as 'determinate',
width: 44,
height: 44,
};
Expand All @@ -42,12 +40,7 @@ addPropertyControls(CircularProgress, {
type: ControlType.Number,
title: 'Value',
hidden: function hidden(props) {
return props.variant === 'indeterminate';
return true;
Copy link
Member Author

Choose a reason for hiding this comment

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

Is this fine? The default for variant is 'indeterminate', that's why I returned true

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, notice how the value widget is gone when the variant is changed:

Capture d’écran 2020-08-13 à 15 27 31

Capture d’écran 2020-08-13 à 15 27 38

Copy link
Member Author

Choose a reason for hiding this comment

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

I reverted this as it was before, but now that we do not have variant select, this will always be hidden, maybe it's better to just show it always (return true)?

Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to change it? Let's say we solve the variant issue, it seems that we will want to keep the same logic.

Copy link
Member Author

@mnajdova mnajdova Aug 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I tried adding @ts-ignore, as well as casting the props as any but anyway the build was failing: https://app.circleci.com/pipelines/github/mui-org/material-ui/16727/workflows/124e27b3-cad3-4e12-b186-301c2e423caa/jobs/169125 :\

Copy link
Member Author

Choose a reason for hiding this comment

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

Locally yarn workspace framer build is failing for me:

$ cross-env BABEL_ENV=test babel-node --config-file ../babel.config.js ./scripts/buildFramer.js ../packages/material-ui-styles/src ./Material-UI.framerfx/code
$ prettier --write --config ../prettier.config.js './**/*.{js,tsx}'
[error] No files matching the pattern were found: "'./**/*.{js,tsx}'".
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 2.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed.
Exit code: 2
Command: C:\Program Files\nodejs\node.exe
Arguments: D:\workspace\material-ui\.yarn\releases\yarn-1.22.4.js build
Directory: D:\workspace\material-ui\framer
Output:

info Visit https://yarnpkg.com/en/docs/cli/workspace for documentation about this command.

Any idea how we can fix this?

},
},
variant: {
type: ControlType.Enum,
title: 'Variant',
options: ['determinate', 'indeterminate'],
},
});
1 change: 1 addition & 0 deletions packages/material-ui-styles/src/makeStyles/makeStyles.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ export default function makeStyles(stylesOrCreator, options = {}) {
'MuiButton',
'MuiButtonGroup',
'MuiChip',
'MuiCircularProgress',
'MuiTypography',
];

Expand Down
9 changes: 8 additions & 1 deletion packages/material-ui/src/CircularProgress/CircularProgress.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import * as React from 'react';
import { OverridableStringUnion } from '@material-ui/types';
import { StandardProps } from '..';

export interface CircularProgressPropsVariantOverrides {}
export type CircularProgressVariantDefaults = Record<'determinate' | 'indeterminate', true>;

export interface CircularProgressProps
extends StandardProps<
React.HTMLAttributes<HTMLDivElement>,
Expand Down Expand Up @@ -35,7 +39,10 @@ export interface CircularProgressProps
* The variant to use.
* Use indeterminate when there is no progress value.
*/
variant?: 'determinate' | 'indeterminate';
variant?: OverridableStringUnion<
CircularProgressVariantDefaults,
CircularProgressPropsVariantOverrides
>;
}

export type CircularProgressClassKey =
Expand Down
23 changes: 20 additions & 3 deletions packages/material-ui/src/CircularProgress/CircularProgress.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as React from 'react';
import PropTypes from 'prop-types';
import clsx from 'clsx';
import { useThemeVariants } from '@material-ui/styles';
import { chainPropTypes } from '@material-ui/utils';
import withStyles from '../styles/withStyles';
import capitalize from '../utils/capitalize';
Expand Down Expand Up @@ -99,6 +100,19 @@ const CircularProgress = React.forwardRef(function CircularProgress(props, ref)
...other
} = props;

const themeVariantsClasses = useThemeVariants(
{
...props,
color,
disableShrink,
size,
thickness,
value,
variant,
},
'MuiCircularProgress',
);

const circleStyle = {};
const rootStyle = {};
const rootProps = {};
Expand All @@ -115,11 +129,11 @@ const CircularProgress = React.forwardRef(function CircularProgress(props, ref)
<div
className={clsx(
classes.root,
classes[variant],
{
[classes[`color${capitalize(color)}`]]: color !== 'inherit',
[classes.determinate]: variant === 'determinate',
[classes.indeterminate]: variant === 'indeterminate',
},
themeVariantsClasses,
className,
)}
style={{ width: size, height: size, ...rootStyle, ...style }}
Expand Down Expand Up @@ -202,7 +216,10 @@ CircularProgress.propTypes = {
* The variant to use.
* Use indeterminate when there is no progress value.
*/
variant: PropTypes.oneOf(['determinate', 'indeterminate']),
variant: PropTypes /* @typescript-to-proptypes-ignore */.oneOfType([
PropTypes.oneOf(['determinate', 'indeterminate']),
PropTypes.string,
]),
};

export default withStyles(styles, { name: 'MuiCircularProgress', flip: false })(CircularProgress);