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

Missing "as" props on @zendeskgarden/react-typography titles #886

Closed
armandabric opened this issue Sep 21, 2020 · 8 comments
Closed

Missing "as" props on @zendeskgarden/react-typography titles #886

armandabric opened this issue Sep 21, 2020 · 8 comments

Comments

@armandabric
Copy link

Expectations

We should be able to change the used HTML tag with the as props. This is a native Styled Component features.

<XXXL as="h1">Hello you!</XXXL> // This should be valid in TS

This code works in JS (the used tag is h1 instead of div) but with TS the validation failed.

Reality

src/foo.tsx:20:19 - error TS2322: Type '{ children: string; as: string; }' is not assignable to type 'IntrinsicAttributes & IXXXLProps & RefAttributes<HTMLDivElement> & { children?: ReactNode; }'.
  Property 'as' does not exist on type 'IntrinsicAttributes & IXXXLProps & RefAttributes<HTMLDivElement> & { children?: ReactNode; }'.

             <XXXL as="h1">Hello you!</XXXL>
                   ~~

Fine Print

  • Version: @zendeskgarden/react-typography v8.0.0
@austingreendev
Copy link
Contributor

Looks like we need to include some additional type extensions for the majority of the components that spread props.

You might be able to get this working locally by following a similar approach to the css prop in styled-components DefinitelyTyped/DefinitelyTyped#31245

Could you give this a shot?

declare module 'react' {
  interface DOMAttributes<T> {
    as?: string;
  }
}

@armandabric
Copy link
Author

It's working in my sandbox application:

Screenshot from 2020-09-23 09-30-38

@austingreendev
Copy link
Contributor

Looking into this more, it looks like styled-components doesn't publicly export any types that we could extend automatically.

They have a private type StyledComponentPropsWithAs

type StyledComponentPropsWithAs<
    C extends keyof JSX.IntrinsicElements | React.ComponentType<any>,
    T extends object,
    O extends object,
    A extends keyof any
> = StyledComponentProps<C, T, O, A> & { as?: C, forwardedAs?: C };

We would have to manually include as?: React.ElementType (or an equivalent) in every component

@jzempel
Copy link
Member

jzempel commented Sep 28, 2020

I could be wrong, but don't we recommend using the tag prop on these particular components? https://garden.zendesk.com/components/typography#md. Having access to as would be nice, too.

@austingreendev
Copy link
Contributor

Good point, the react-typography elements should have access to that prop in TS. The as typing listed above could be used with other components that spread props to a styled element.

@armandabric
Copy link
Author

armandabric commented Oct 1, 2020

Having two different props (as and tag) will be confusing. It could be a good think to harmonize and only use one

@austingreendev
Copy link
Contributor

Some background on react-typography, we had the tag prop before moving to styled-components, which is what allows the as prop.

@jzempel after some more thought I'm not sure if we should document this further for the non-typography elements. It makes sense for the typography use-case, but could lead to a11y issues for other components if consumers started to tweak them.

If we were using native-focus for react-dropdowns allowing anchor menu items with Item tag="a" could be another good use-case.

@jzempel
Copy link
Member

jzempel commented Feb 8, 2021

The decision is to leave this as-is. Use tag on typography components in order to modify the rendered DOM element.

@jzempel jzempel closed this as completed Feb 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants