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.

Sign up for GitHub

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

Jump to bottom

[select] feat(Select): new prop "fill" #4843

Merged
merged 5 commits into from
Aug 24, 2021
Merged

[select] feat(Select): new prop "fill" #4843

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6baaa2e
Fill option for select
millasml Aug 5, 2021
d093368
use React.Children.map in case there is more than 1 child
millasml Aug 5, 2021
9c5e019
Remove react cloning element
millasml Aug 16, 2021
4fbe281
Merge remote-tracking branch 'origin/develop' into ms/select-fill
adidahiya Aug 24, 2021
36580fd
update example to work correctly
adidahiya Aug 24, 2021
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
5 changes: 5 additions & 0 deletions packages/docs-app/src/examples/select-examples/selectExample.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export interface ISelectExampleState {
resetOnSelect: boolean;
disableItems: boolean;
disabled: boolean;
fill: boolean;
}

export class SelectExample extends React.PureComponent<IExampleProps, ISelectExampleState> {
Expand All @@ -43,6 +44,7 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa
createdItems: [],
disableItems: false,
disabled: false,
fill: false,
filterable: true,
hasInitialContent: false,
minimal: false,
Expand All @@ -57,6 +59,8 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa

private handleDisabledChange = this.handleSwitchChange("disabled");

private handleFillChange = this.handleSwitchChange("fill");

private handleFilterableChange = this.handleSwitchChange("filterable");

private handleInitialContentChange = this.handleSwitchChange("hasInitialContent");
Expand Down Expand Up @@ -114,6 +118,7 @@ export class SelectExample extends React.PureComponent<IExampleProps, ISelectExa
checked={this.state.resetOnSelect}
onChange={this.handleResetOnSelectChange}
/>
<Switch label="Fill container width" checked={this.state.fill} onChange={this.handleFillChange} />
<Switch
label="Use initial content"
checked={this.state.hasInitialContent}
Expand Down
21 changes: 19 additions & 2 deletions packages/select/src/components/select/select.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ import { IQueryListRendererProps, QueryList } from "../query-list/queryList";
export type SelectProps<T> = ISelectProps<T>;
/** @deprecated use SelectProps */
export interface ISelectProps<T> extends IListItemsProps<T> {
/**
* Whether the component should take up the full width of its container.
* This overrides `popoverProps.fill` and `tagInputProps.fill`.
adidahiya marked this conversation as resolved.
Show resolved Hide resolved
*/
fill?: boolean;

/**
* Whether the dropdown list can be filtered.
* Disabling this option will remove the `InputGroup` and ignore `inputProps`.
Expand Down Expand Up @@ -129,7 +135,18 @@ export class Select<T> extends AbstractPureComponent2<SelectProps<T>, ISelectSta

private renderQueryList = (listProps: IQueryListRendererProps<T>) => {
// not using defaultProps cuz they're hard to type with generics (can't use <T> on static members)
const { filterable = true, disabled = false, inputProps = {}, popoverProps = {} } = this.props;
const { fill, filterable = true, disabled = false, inputProps = {}, popoverProps = {} } = this.props;

const childProps: { fill?: boolean } = {};

if (fill) {
popoverProps.fill = true;
childProps.fill = true;
}

const children = React.Children.map(this.props.children, (child: React.ReactNode) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't love this approach and would prefer not to clone children if we can avoid it. We can't guarantee that the children will support the fill prop, and I'd rather not send along extraneous HTML attributes which might get rendered out to the DOM (and produce a React warning).

If we stick to just rendering {this.props.children}, then <Select> users will have to add fill to children themselves (or style it some other way), but I think that's ok. We should make a note of that in the fill prop documentation.

return React.cloneElement(child as React.ReactElement, { ...childProps });
});

const input = (
<InputGroup
Expand Down Expand Up @@ -164,7 +181,7 @@ export class Select<T> extends AbstractPureComponent2<SelectProps<T>, ISelectSta
onKeyDown={this.state.isOpen ? handleKeyDown : this.handleTargetKeyDown}
onKeyUp={this.state.isOpen ? handleKeyUp : undefined}
>
{this.props.children}
{children}
</div>
<div onKeyDown={handleKeyDown} onKeyUp={handleKeyUp}>
{filterable ? input : undefined}
Expand Down