Skip to content

Commit

Permalink
[Typography] Improve custom component types support (#18868)
Browse files Browse the repository at this point in the history
  • Loading branch information
@fyodore82 @oliviertassinari
fyodore82 authored and oliviertassinari committed Dec 17, 2019
1 parent c77c2e8 commit b6ff4f5
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 20 deletions.
48 changes: 28 additions & 20 deletions packages/material-ui/src/Typography/Typography.d.ts
View file
Original file line number Diff line number Diff line change
@@ -1,29 +1,39 @@
import * as React from 'react';
import { StandardProps, PropTypes } from '..';
import { OverrideProps, OverridableTypeMap, OverridableComponent } from '../OverridableComponent';
import { ThemeStyle } from '../styles/createTypography';

type Style = ThemeStyle | 'srOnly';

export interface TypographyProps
extends StandardProps<React.HTMLAttributes<HTMLElement>, TypographyClassKey> {
align?: PropTypes.Alignment;
color?:
| 'initial'
| 'inherit'
| 'primary'
| 'secondary'
| 'textPrimary'
| 'textSecondary'
| 'error';
component?: React.ElementType<React.HTMLAttributes<HTMLElement>>;
display?: 'initial' | 'block' | 'inline';
gutterBottom?: boolean;
noWrap?: boolean;
paragraph?: boolean;
variant?: Style | 'inherit';
variantMapping?: Partial<Record<Style, string>>;
export interface TypographyTypeMap<P = {}, D extends React.ElementType = 'span'> {
props: P & {
align?: PropTypes.Alignment;
color?:
| 'initial'
| 'inherit'
| 'primary'
| 'secondary'
| 'textPrimary'
| 'textSecondary'
| 'error';
display?: 'initial' | 'block' | 'inline';
gutterBottom?: boolean;
noWrap?: boolean;
paragraph?: boolean;
variant?: Style | 'inherit';
variantMapping?: Partial<Record<Style, string>>;
};
defaultComponent: D;
classKey: TypographyClassKey;
}

declare const Typography: OverridableComponent<TypographyTypeMap>;

export type TypographyProps<
D extends React.ElementType = TypographyTypeMap['defaultComponent'],
P = {}
> = OverrideProps<TypographyTypeMap<P, D>, D>;

export type TypographyClassKey =
| 'root'
| 'h1'
Expand Down Expand Up @@ -54,6 +64,4 @@ export type TypographyClassKey =
| 'displayInline'
| 'displayBlock';

declare const Typography: React.ComponentType<TypographyProps>;

export default Typography;
39 changes: 39 additions & 0 deletions packages/material-ui/src/Typography/typography.spec.tsx
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import React, { FC } from 'react';
import { Typography } from '@material-ui/core';

const TypographyTest = () => {
const CustomComponent: React.FC<{ prop1: string; prop2: number }> = () => <div />;
return (
<div>
<Typography />
<Typography align="inherit" color="inherit" display="block" />
<Typography align="left" color="initial" display="inline" />
<Typography align="right" color="primary" display="initial" />
<Typography align="justify" color="secondary" display="initial" />
<Typography align="inherit" color="textPrimary" />
<Typography align="inherit" color="textSecondary" />
<Typography align="inherit" color="error" />
// $ExpectError
<Typography display="incorrectValue" />
<Typography component="a" href="url" display="block" />
<Typography component="label" htmlFor="html" display="block" />
// $ExpectError
<Typography component="a" href="url" display="incorrectValue" />
// $ExpectError
<Typography component="a" incorrectAttribute="url" />
// $ExpectError
<Typography component="incorrectComponent" href="url" />
// $ExpectError
<Typography component="div" href="url" />
// $ExpectError
<Typography href="url" />
<Typography component={CustomComponent} prop1="1" prop2={12} />
// $ExpectError
<Typography component={CustomComponent} prop1="1" prop2={12} id="1" />
// $ExpectError
<Typography component={CustomComponent} prop1="1" />
// $ExpectError
<Typography component={CustomComponent} prop1="1" prop2="12" />
</div>
);
};

4 comments on commit b6ff4f5

@codingedgar
Copy link

Choose a reason for hiding this comment

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

@fyodore82 @oliviertassinari this change is not in the docs, took quite a while to find the change.

@fyodore82
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@edgarjrg , please take a look here.
It is pretty well described.

@rart
Copy link
Contributor

@rart rart commented on b6ff4f5 Mar 12, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

@fyodore82 @oliviertassinari in version v4.7.2, the below used to work:

<CardHeader
  title="My Title"
  subheader="My Subheader"
  titleTypographyProps={{ component: "h2"}}
  subheaderTypographyProps={{ component: "p" }}
/>

Somewhere after (4.8.3 o 4.9.5), it broke due to this PR removing the component as an explicit prop of Typography.

Was that a planned breaking change?
I reckon we'd need to do something like below to get that back to working. Happy to submit a PR if you guys think that's the right approach; otherwise, can you please suggest how to use component prop for Typography on the CardHeader? Also, is there any other component that has the typography props that would need to get updated?

export interface CardHeaderProps extends StandardProps<React.HTMLAttributes<HTMLDivElement>, CardHeaderClassKey, 'title'> {
  ...
  subheaderTypographyProps?: Partial<TypographyProps<any, { component: unknown }>>;
  ...
  titleTypographyProps?: Partial<TypographyProps<any, { component: unknown }>>;
}

To speed up things, I went ahead and opened a PR

@fyodore82
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rart , this is related to all components which depends on TypographyProps. About a month ago I've opened the pull request to fix this for ListItemText. But it is still under review.
If it will be merged, the same solution can be applied to all other components (including CardHeader

Please sign in to comment.