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

Refactor: Utilisation de TypeScript #51

Merged
merged 42 commits into from
Aug 21, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
53fb5c8
buttons and inputs
Aug 9, 2019
4a48d81
Forms
Aug 9, 2019
4fc5495
Headband
Aug 9, 2019
3ea90dd
Other components
Aug 12, 2019
0bde61c
Fixes
Aug 12, 2019
2671787
TSC fixes
Aug 13, 2019
ecf4667
PR fixes
Aug 13, 2019
e2973e6
TS fixes
Aug 13, 2019
5f93e05
PR fixes
Aug 14, 2019
2f2fbad
PR fixes
Aug 14, 2019
c355ec0
PR fixes
Aug 15, 2019
f0dd2f4
Merge Master
Aug 15, 2019
8a34eb3
New components refactor
Aug 15, 2019
a8bcc35
TS Typings Core setup
Aug 15, 2019
1fd91d9
buttons and inputs
Aug 9, 2019
0064990
Forms
Aug 9, 2019
b5740bb
Headband
Aug 9, 2019
ee0f877
Other components
Aug 12, 2019
9579097
Fixes
Aug 12, 2019
408c732
TSC fixes
Aug 13, 2019
cfd8c61
PR fixes
Aug 13, 2019
e95df60
TS fixes
Aug 13, 2019
71e20d5
PR fixes
Aug 14, 2019
5c5fc1f
PR fixes
Aug 14, 2019
c0ee19a
PR fixes
Aug 15, 2019
d23f445
New components refactor
Aug 15, 2019
5ce285d
TS Typings Core setup
Aug 15, 2019
2472272
PR fixes
Aug 20, 2019
6e613e6
Pull
Aug 20, 2019
4b2ad39
wip
christian-roy Aug 20, 2019
f11943d
renames Select.tsx to select.tsx
christian-roy Aug 20, 2019
2b72427
PR fixes
Aug 20, 2019
8ca26c4
Rollup config/plugin update
Aug 21, 2019
e4905b1
TypeScript in chooser.tsx
Aug 21, 2019
35215b2
Rollup changes fixes
Aug 21, 2019
dffa255
Changed Child exports/imports for ReactNode
Aug 21, 2019
c6e8745
Fixes chooser.tsx
Aug 21, 2019
6be2984
package.json update
Aug 21, 2019
c85e33b
styles imports syntax update
Aug 21, 2019
f000ba3
progress-bar.tsx update
Aug 21, 2019
69e2730
config.js update
Aug 21, 2019
2f36bd5
Review Fixes
christian-roy Aug 21, 2019
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
14 changes: 0 additions & 14 deletions packages/react/src/components/a11y/visuallyhidden.jsx

This file was deleted.

17 changes: 17 additions & 0 deletions packages/react/src/components/a11y/visuallyhidden.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import React from 'react';
import styled from 'styled-components';

import { Children } from '../buttons/abstract-button';
import styles from './styles/visuallyhidden';

const Hidden = styled.span`
${styles}
`;

const VisuallyHidden = ({ children }: {children: Children}) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Pour tous les functional components, faire une interface pas exportée (sauf si nécessaire) pour les props et changer les const Component = () => ReactElement pour export function Component(): ReactElement

interface Props {
    children: ReactNode;
}

export function VisuallyHidden({ children }: Props): ReactElement {
    return (
        <Hidden>
            {children}
        </Hidden>
    );
}

<Hidden>
{children}
</Hidden>
);

export { VisuallyHidden };
4 changes: 3 additions & 1 deletion packages/react/src/components/buttons/abstract-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@ import styled from 'styled-components';

import abstractStyle from './styles/abstract';

export type Children = ReactNode[] | string | Element;

export interface AbstractButtonProps {
children?: ReactNode[];
children?: Children;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tu peux utiliser simplement ReactNode, c'est tout ce que React supporte comme children

disabled?: boolean;
onClick(): void;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/buttons/button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import tertiaryStyle from './styles/tertiary';

import { AbstractButton, AbstractButtonProps } from './abstract-button';

export type ButtonType = 'primary' | 'secondary' | 'tertiary';
type ButtonType = 'primary' | 'secondary' | 'tertiary';

export interface ButtonProps extends AbstractButtonProps {
buttonType: ButtonType;
Expand Down
2 changes: 1 addition & 1 deletion packages/react/src/components/buttons/search-button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import styled from 'styled-components';

import { AbstractButton, AbstractButtonProps } from './abstract-button';

export interface SearchButtonProps extends AbstractButtonProps {
interface SearchButtonProps extends AbstractButtonProps {
className: string;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ const Card = styled.div`
padding: 3rem 2rem;
`;

export default Card;
export { Card };
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ const EnsoSpinner = styled(Enso)`
}
`;

export default EnsoSpinner;
export { EnsoSpinner };
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ const Field = styled.div`
margin: 0.25rem 0 0;
`;

const InvalidField = ({ controlId, feedbackMsg }) => (
interface InvalidFieldProps {
controlId: string;
feedbackMsg: string;
}

const InvalidField = ({ controlId, feedbackMsg }: InvalidFieldProps) => (
<Field
role="alert"
aria-live="polite"
Expand All @@ -20,4 +25,4 @@ const InvalidField = ({ controlId, feedbackMsg }) => (
</Field>
);

export default InvalidField;
export { InvalidField };
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import React from 'react';
import styled from 'styled-components';

import Label from './label';
import InvalidField from '../feedbacks/invalid-field';
import { Children } from '../buttons/abstract-button';
import { InvalidField } from '../feedbacks/invalid-field';
import { Label } from './label';

const StyledDiv = styled.div`
margin: 0 0 1.5rem;
Expand All @@ -18,7 +19,16 @@ const StyledDiv = styled.div`
}
`;

const FieldContainer = ({ children, fieldId, label, valid, validMsg, ...props }) => (
interface FieldContainerProps {
children: Children;
Copy link
Contributor

Choose a reason for hiding this comment

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

Je pense que le type Children pourrait être supprimé et seulement utiliser ReactNode à la place

fieldId: string;
label: string;
valid: boolean;
validMsg: string;

Copy link
Contributor

Choose a reason for hiding this comment

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

Ligne vide

}

const FieldContainer = ({ children, fieldId, label, valid, validMsg, ...props }: FieldContainerProps) => (
<StyledDiv {...props} valid={valid}>
{label && (
<Label forId={fieldId}>
Expand All @@ -34,4 +44,4 @@ const FieldContainer = ({ children, fieldId, label, valid, validMsg, ...props })
</StyledDiv>
);

export default FieldContainer;
export { FieldContainer } ;
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from 'react';
import React, { RefObject } from 'react';
import styled from 'styled-components';

const Input = styled.input`
Expand All @@ -7,8 +7,15 @@ const Input = styled.input`
width: 16px;
`;

const Checkbox = ({ defaultChecked, onChange }) => {
const ref = React.createRef();
interface CheckboxProps {
defaultChecked?: boolean;
onChange: ((...args: any[]) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

onChange(value: boolean): void

}

type Ref = ((instance: HTMLInputElement) => void) | RefObject<HTMLInputElement>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Les parenthèses sont peut-être de trop autour de ((instance: HTMLInputElement) => void)


const Checkbox = ({ defaultChecked, onChange }: CheckboxProps) => {
const ref: Ref = React.createRef();
Copy link
Contributor

Choose a reason for hiding this comment

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

useRef<HTMLInputElement>(null) plutôt que React.createRef, mais c'est peut-être déjà dans votre backlog de passer au hooks


const handleChange = () => {
if (typeof onChange === 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Plus besoin de faire cette validation en TypeScript.

Expand All @@ -21,4 +28,4 @@ const Checkbox = ({ defaultChecked, onChange }) => {
);
};

export default Checkbox;
export { Checkbox };
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,18 @@ const Input = styled.input`
}
`;

const OptionButton = ({ checked, label, name, value }) => (
interface OptionButtonProps {
checked?: boolean;
label: string;
name: string;
value: number;
}

const OptionButton = ({ checked, label, name, value }: OptionButtonProps) => (
Copy link
Contributor

Choose a reason for hiding this comment

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

Rien à faire pour le review, mais ce component n'est pas utilisé nul part et n'a pas de story correspondante dans Storybook.

<div>
<Input checked={checked} id="id" name={name} type="radio" value={value} />
<label htmlFor="id">{label}</label>
</div>
);

export default OptionButton;
export { OptionButton };

This file was deleted.

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

import { SearchInput, SearchInputProps } from './search-input';

const SearchContextual = ({ disabled, id, label, onChange }: SearchInputProps) => (
<SearchInput
disabled={disabled}
id={id}
label={label}
onChange={onChange}
/>
);

export { SearchContextual };
15 changes: 0 additions & 15 deletions packages/react/src/components/forms/inputs/search-global.jsx

This file was deleted.

14 changes: 14 additions & 0 deletions packages/react/src/components/forms/inputs/search-global.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import React from 'react';
import { SearchInput, SearchInputProps } from './search-input';

const SearchGlobal = ({ disabled, id, label, onSearch }: SearchInputProps) => (
<SearchInput
disabled={disabled}
hasButton
id={id}
label={label}
onSearch={onSearch}
/>
);

export { SearchGlobal };
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import React, { useState } from 'react';
import styled from 'styled-components';

import SearchIcon from 'feather-icons/dist/icons/search.svg';
import XIcon from 'feather-icons/dist/icons/x.svg';
import styled from 'styled-components';

import VisuallyHidden from '../../a11y/visuallyhidden';
import { VisuallyHidden } from '../../a11y/visuallyhidden';
import { SearchButton } from '../../buttons/search-button';

import { Label } from '../label';
import style from '../styles/inputs';
import Label from '../label';

const SearchWrapper = styled.div`
display: flex;
Expand Down Expand Up @@ -86,10 +87,19 @@ const SearchSubmit = styled(SearchButton)`
position: relative;
`;

const SearchInput = ({ disabled, hasButton, id, label, onChange, onSearch }) => {
export interface SearchInputProps {
disabled?: boolean;
hasButton?: boolean;
id: string;
label: string;
onChange?: ((...args: any[]) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

onChange?(value: string): void

onSearch?: ((...args: any[]) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

onSearch?(value: string): void

}

const SearchInput = ({ disabled, hasButton, id, label, onChange, onSearch }: SearchInputProps) => {
const [{ value }, setValue] = useState({ value: '' });

const handleChange = event => {
const handleChange = (event: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

event: ChangeEvent<HTMLInputElement> (import { ChangeEvent} from 'react')

Copy link
Contributor

Choose a reason for hiding this comment

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

Et c'était déjà de même, mais pas besoin de faire onChange={event => handleChange(event)} dans l'Input. Tu pourrais faire directement handleChange={handleChange} (même chose pour onKeyDown)

const newValue = event.target.value;
setValue({ value: newValue });

Expand All @@ -98,7 +108,7 @@ const SearchInput = ({ disabled, hasButton, id, label, onChange, onSearch }) =>
}
};

const handleKeyDown = event => {
const handleKeyDown = (event: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

event: KeyboardEvent<HTMLInputElement> (import { KeyboardEvent } from 'react')

if (typeof onSearch === 'function' && event.keyCode === 13) {
onSearch(value);
}
Expand Down Expand Up @@ -154,4 +164,4 @@ const SearchInput = ({ disabled, hasButton, id, label, onChange, onSearch }) =>
);
};

export default SearchInput;
export { SearchInput };
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import styled from 'styled-components';

import style from '../styles/inputs';

import FieldContainer from '../field-container';
import { FieldContainer } from '../field-container';

const StyledTextArea = styled.textarea`
${style}
Expand All @@ -14,11 +14,23 @@ const StyledTextArea = styled.textarea`
resize: vertical;
`;

const TextArea = ({ defaultValue, disabled, id, label, onBlur, onChange, onFocus, required, validMsg }) => {
export interface TextAreaProps {
defaultValue?: string;
disabled?: boolean;
id: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

Me semble que le id devrait être facultatif

Copy link
Contributor

Choose a reason for hiding this comment

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

Ouin on le met pour lier le label à l'input, mais en réalité ça devrait être fait à l'intérieur de la lib et pas exposé dans une prop.

Je vais créer une carte et on fera le refactor plus tard.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sinon l'autre possibilité c'est de mettre les inputs dans le tag label, comme ça t'as pas besoin du htmlFor :

<label>
    Allo
   <input />
</label

Par contre faudrait retravailler le css, je pense qu'il est fait pour que le label soit à côté du input. Mais ça pourrait permettre d'avoir un id facultatif, si quelqu'un en a besoin, et que les deux soient liés quand même.

label: string;
onBlur?: ((...args: any[]) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

onBlur?(value: string): void;

onChange?: ((...args: any[]) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

onChange?(value: string): void;

onFocus?: ((...args: any[]) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

onFocus?(value: string): void;

required?: boolean;
validMsg?: string;
}

const TextArea = ({ defaultValue, disabled, id, label, onBlur, onChange, onFocus, required, validMsg }: TextAreaProps) => {
const [{ value }, setValue] = useState({ value: defaultValue || '' });
const [{ validity }, setValidity] = useState({ validity: true });

const handleBlur = event => {
const handleBlur = (event: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

event: FocusEvent<HTMLTextAreaElement>

const newValue = event.target.value;

setValue({ value: newValue });
Expand All @@ -29,7 +41,7 @@ const TextArea = ({ defaultValue, disabled, id, label, onBlur, onChange, onFocus
}
};

const handleChange = event => {
const handleChange = (event: any) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

event: ChangeEvent<HTMLTextAreaElement>

const newValue = event.target.value;
setValue({ value: newValue });

Expand All @@ -56,12 +68,12 @@ const TextArea = ({ defaultValue, disabled, id, label, onBlur, onChange, onFocus
id={id}
onBlur={event => handleBlur(event)}
onChange={event => handleChange(event)}
onFocus={event => handleFocus(event)}
onFocus={event => handleFocus()}
Copy link
Contributor

Choose a reason for hiding this comment

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

Mettre les fonctions directement plutôt que event => method(event)

required={required}
value={value}
/>
</FieldContainer>
);
};

export default TextArea;
export { TextArea };
Loading