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

Add typescript definitions for more components #326

Merged
merged 3 commits into from
Jan 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
- Added `EuiColorPicker`. ((328)[https://github.com/elastic/eui/pull/328])
- `EuiCodeBlock` now only shows fullscreen icons if `overflowHeight` prop is set. Also forces large fonts and padding while expanded. [(#325)](https://github.com/elastic/eui/pull/325)
- Exported `VISUALIZATION_COLORS` from services ((#329)[https://github.com/elastic/eui/pull/329])
- Add typescript definitions for `<EuiFormRow>`, `<EuiRadioGroup>`, `<EuiSwitch>`, `<EuiLoadingSpinner>`, `<EuiLoadingChart>` and `<EuiProgress>`. [(#326)](https://github.com/elastic/eui/pull/326)

**Breaking changes**

- `EuiCodeBlock` now only shows fullscreen icons if `overflowHeight` prop is set. Also forces large fonts and padding while expanded. [(#325)](https://github.com/elastic/eui/pull/325)
- React ^16.2 is now a peer dependency ([#264](https://github.com/elastic/eui/pull/264))
- `<EuiProgress>` no longer accepts the `indeterminate` property, which never had any effect. [(#326)](https://github.com/elastic/eui/pull/326)

**Bug fixes**

Expand Down
21 changes: 21 additions & 0 deletions src/components/form/form_row/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
/// <reference path="../../common.d.ts" />

import { SFC, ReactNode, HTMLAttributes } from 'react';

declare module '@elastic/eui' {
/**
* @see './form_row.js'
*/

export type EuiFormRowProps = CommonProps &
HTMLAttributes<HTMLDivElement> & {
error?: string | string[];
fullWidth?: boolean;
hasEmptyLabelSpace?: boolean;
helpText?: ReactNode;
isInvalid?: boolean;
label?: ReactNode;
};

export const EuiFormRow: SFC<EuiFormRowProps>;
}
3 changes: 3 additions & 0 deletions src/components/form/index.d.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
/// <reference path="./checkbox/index.d.ts" />
/// <reference path="./field_search/index.d.ts" />
/// <reference path="./form_row/index.d.ts" />
/// <reference path="./radio/index.d.ts" />
/// <reference path="./switch/index.d.ts" />
26 changes: 26 additions & 0 deletions src/components/form/radio/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/// <reference path="../../common.d.ts" />

import { SFC, HTMLAttributes, ReactNode } from 'react';

declare module '@elastic/eui' {
/**
* @see './radio_group.js'
*/
export interface EuiRadioGroupOption {
id: string;
label?: ReactNode;
}

export type EuiRadioGroupChangeCallback = (id: string) => void;

export type EuiRadioGroupProps = CommonProps &
Omit<HTMLAttributes<HTMLDivElement>, 'onChange'> & {
options?: EuiRadioGroupOption[];
Copy link
Contributor

Choose a reason for hiding this comment

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

should be required

Copy link
Member Author

Choose a reason for hiding this comment

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

It is listed in defaultProps though, which suggests the intention is for it to be optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

good point, I didn't see the default props... it's still marked as a required propType (not sure how that behaves along with the default prop... would it still shout if one is not supplied?). In any case, we need to have a decision - it's either required or optional with a default... it make little sense to be both :)

/cc @snide ^

Copy link
Member Author

Choose a reason for hiding this comment

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

The defaultProps lead to isRequired not shouting.

idSelected?: string;
onChange: EuiRadioGroupChangeCallback;
};

export type x = EuiRadioGroupProps['onChange'];

export const EuiRadioGroup: SFC<EuiRadioGroupProps>;
}
18 changes: 18 additions & 0 deletions src/components/form/switch/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/// <reference path="../../common.d.ts" />

import { SFC, InputHTMLAttributes, ReactNode } from 'react';

declare module '@elastic/eui' {
/**
* @see './switch.js'
*/
export type EuiSwitchChangeCallback = (state: boolean) => void;

export type EuiSwitchProps = CommonProps &
Omit<InputHTMLAttributes<HTMLInputElement>, 'onChange'> & {
label?: ReactNode;
onChange?: EuiSwitchChangeCallback;
};

export const EuiSwitch: SFC<EuiSwitchProps>;
}
2 changes: 2 additions & 0 deletions src/components/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,5 @@
/// <reference path="./spacer/index.d.ts" />
/// <reference path="./pagination/index.d.ts" />
/// <reference path="./link/index.d.ts" />
/// <reference path="./loading/index.d.ts" />
/// <reference path="./progress/index.d.ts" />
30 changes: 30 additions & 0 deletions src/components/loading/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/// <reference path="../common.d.ts" />

import { SFC, HTMLAttributes } from 'react';

declare module '@elastic/eui' {
/**
* @see './loading_spinner.js'
*/
export type EuiLoadingSpinnerSize = 's' | 'm' | 'l' | 'xl';

export type EuiLoadingSpinnerProps = CommonProps &
HTMLAttributes<HTMLDivElement> & {
size?: EuiLoadingSpinnerSize;
};

export const EuiLoadingSpinner: SFC<EuiLoadingSpinnerProps>;

/**
* @see './loading_chart.js'
*/
export type EuiLoadingChartSize = 'm' | 'l' | 'xl';

export type EuiLoadingChartProps = CommonProps &
HTMLAttributes<HTMLDivElement> & {
mono?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we take this opportunity to also add mono to the propTypes?

size?: EuiLoadingChartSize;
};

export const EuiLoadingChart: SFC<EuiLoadingChartProps>;
}
6 changes: 5 additions & 1 deletion src/components/loading/loading_chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ export const EuiLoadingChart = ({ size, mono, className, ...rest }) => {
};

EuiLoadingChart.propTypes = {
size: PropTypes.oneOf(SIZES),
mono: PropTypes.bool,
size: PropTypes.oneOf(SIZES)
};

EuiLoadingChart.defaultProps = {
mono: false
};
29 changes: 29 additions & 0 deletions src/components/progress/index.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/// <reference path="../common.d.ts" />

import { SFC, HTMLAttributes } from 'react';

declare module '@elastic/eui' {
/**
* @see './progress.js'
*/
export type EuiProgressColor =
| 'accent'
| 'danger'
| 'primary'
| 'secondary'
| 'subdued';

export type EuiProgressSize = 'xs' | 's' | 'm' | 'l';

export type EuiProgressPosition = 'fixed' | 'absolute' | 'static';

export type EuiProgressProps = CommonProps &
HTMLAttributes<HTMLProgressElement> & {
size?: EuiProgressSize;
color?: EuiProgressColor;
position?: EuiProgressPosition;
max?: number;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we use this opportunity to remove unused indeterminate from propTypes?

Copy link
Member Author

@weltenwort weltenwort Jan 22, 2018

Choose a reason for hiding this comment

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

Looks like that should be a safe change. The tests did not cover it anyway. @snide, any objections?

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeterminate progress bars were still being used last time I checked. Why do you want to remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

The property does not seem to be used anywhere in the component itself. It is passed on to the <progress /> (as part of {...rest}), but the html specs don't recognize that attribute either. Instead the react code interprets the lack of a max property as being indeterminate and the html spec uses the value attribute for that. That's something we might want to align as well.


export const EuiProgress: SFC<EuiProgressProps>;
}
1 change: 0 additions & 1 deletion src/components/progress/progress.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@ EuiProgress.propTypes = {
color: PropTypes.oneOf(COLORS),
position: PropTypes.oneOf(POSITIONS),
max: PropTypes.number,
indeterminate: PropTypes.bool,
};

EuiProgress.defaultProps = {
Expand Down