Disabled prop #1277

[clintonlunn]

Before submitting a pull request, please make sure the following is done:

1. Fork the repository and create your branch from master.
2. Run yarn in the repository root.
3. If you've fixed a bug or added code that should be tested, add tests!
4. Ensure the test suite passes (yarn test). Tip: yarn test --watch TestName is helpful in development.
5. Make sure your code passed the conventional commits check. Read more about conventional commits

[clintonlunn]

I added the same props to both form-upload and upload packages. It looks like what was broken in my example too. I haven't updated the stories just yet, but I can with whatever needs to be put in there.

[codecov[bot]]

Codecov Report

Merging #1278 (c5acc5e) into master (0675bb4) will decrease coverage by 6.35%. The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1278      +/-   ##
==========================================
- Coverage   91.53%   85.19%   -6.35%     
==========================================
  Files          51       16      -35     
  Lines        1631      581    -1050     
  Branches      488      140     -348     
==========================================
- Hits         1493      495     -998     
+ Misses        138       86      -52     

see 51 files with indirect coverage changes

[bjnewman]

isn't this the same issue for any html property? i think the interfaces should extend the base React button interface to inherit all those properties? See https://mortenbarklund.com/blog/react-typescript-props-spread/ for example

[clintonlunn]

isn't this the same issue for any html property? i think the interfaces should extend the base React button interface to inherit all those properties? See https://mortenbarklund.com/blog/react-typescript-props-spread/ for example

Thanks for the comment. That makes sense. In that case, I also get errors about onClick and onChange,I am assuming we would also want to pass those along from a base type.

i.e. maybe we'd want to do something like this:

import type { ButtonHTMLAttributes, ComponentPropsWithoutRef } from 'react';

export interface FilePickerBtnProps extends ComponentPropsWithoutRef<'button'> {
  multiple?: boolean;
  name?: string;
  color?: string;
  children?: React.ReactType;
  allowedFileTypes?: string[];
  maxSize?: number;
  'data-testid'?: string;
}

or

import type { ButtonHTMLAttributes, ComponentPropsWithoutRef } from 'react';

export interface FilePickerBtnProps extends ButtonHTMLAttributes<HTMLButtonElement> {
  multiple?: boolean;
  name?: string;
  color?: string;
  children?: React.ReactType;
  allowedFileTypes?: string[];
  maxSize?: number;
  'data-testid'?: string;
}

[clintonlunn]

I can't quite get the type for onChange to pass through for the FilePicker component unless i do something like export interface FilePickerProps<T extends React.ElementType = 'input'> {... where i define a default react elementtype or if i make the on change something like onChange?: React.ChangeEventHandler<HTMLInputElement>. It seems like the tag can be a number of different types of elements, so passing a correct onchange would mean that we would want to keep as generic as possible?

[bjnewman]

I can't quite get the type for onChange to pass through for the FilePicker component unless i do something like export interface FilePickerProps<T extends React.ElementType = 'input'> {... where i define a default react elementtype or if i make the on change something like onChange?: React.ChangeEventHandler<HTMLInputElement>. It seems like the tag can be a number of different types of elements, so passing a correct onchange would mean that we would want to keep as generic as possible?

the onChange gets passed to the FilePicker so it should have the same type as FIlePicker's onChange which is, yeah a little wider than React.ChangeEventHandler\ since the input tag can be overridden. Does React.ChangeEventHandler<HTMLElement | React.ComponentType> or something similarly broad work?

[bjnewman]

I can't quite get the type for onChange to pass through for the FilePicker component unless i do something like export interface FilePickerProps<T extends React.ElementType = 'input'> {... where i define a default react elementtype or if i make the on change something like onChange?: React.ChangeEventHandler<HTMLInputElement>. It seems like the tag can be a number of different types of elements, so passing a correct onchange would mean that we would want to keep as generic as possible?

the onChange gets passed to the FilePicker so it should have the same type as FIlePicker's onChange which is, yeah a little wider than React.ChangeEventHandler since the input tag can be overridden. Does React.ChangeEventHandler<HTMLElement | React.ComponentType> or something similarly broad work?

After talking to Clinton I think something more like should be good enough for our onChange type without getting too generic

[clintonlunn]

@bjnewman i coudn't get your suggestion above to type things correctly so i went with a more simplistic approach of declaring only the onchange as a React.ChangeEventHandler<HTMLInputElement> and everything else will get either custom typed or inherited