Skip to content

Commit

Permalink
[refactor] Remove need for placeholder text el by using the actual …
Browse files Browse the repository at this point in the history
…`placeholder` attr on the input component
  • Loading branch information
cee-chen committed Nov 1, 2023
1 parent 4505808 commit 32667ac
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 32 deletions.
11 changes: 5 additions & 6 deletions src/components/combo_box/_combo_box.scss
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,11 @@

/* 3 */
background: transparent;
color: $euiTextColor;
@include euiFormControlText;

&:disabled {
@include euiFormControlDisabledStyle;
}

/* 4 */
// stylelint-disable declaration-no-important
Expand Down Expand Up @@ -121,11 +125,6 @@
-webkit-text-fill-color: unset; // overrides $euiFormControlDisabledColor because the color doesn't work with multiple background colors
}

.euiComboBoxPlaceholder,
.euiComboBoxPill--plainText {
@include euiFormControlDisabledTextStyle;
}

// overrides the `cursor: text;` that displays on hover for combo boxes that allow multiple pills
.euiComboBox__inputWrap:not(.euiComboBox__inputWrap--noWrap):hover {
cursor: not-allowed;
Expand Down
39 changes: 39 additions & 0 deletions src/components/combo_box/combo_box.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,45 @@ describe('EuiComboBox', () => {
});
});

describe('placeholder', () => {
it('renders', () => {
const { getByTestSubject } = render(
<EuiComboBox
options={options}
selectedOptions={[]}
placeholder="Select something"
/>
);
const searchInput = getByTestSubject('comboBoxSearchInput');

expect(searchInput).toHaveAttribute('placeholder', 'Select something');
expect(searchInput).toHaveStyle('inline-size: 100%');
});

it('does not render the placeholder if a selection has been made', () => {
const { getByTestSubject } = render(
<EuiComboBox
options={options}
selectedOptions={[options[0]]}
placeholder="Select something"
/>
);
const searchInput = getByTestSubject('comboBoxSearchInput');
expect(searchInput).not.toHaveAttribute('placeholder');
});

it('does not render the placeholder if a search value exists', () => {
const { getByTestSubject } = render(
<EuiComboBox options={options} placeholder="Select something" />
);
const searchInput = getByTestSubject('comboBoxSearchInput');
expect(searchInput).toHaveAttribute('placeholder');

fireEvent.change(searchInput, { target: { value: 'some search' } });
expect(searchInput).not.toHaveAttribute('placeholder');
});
});

test('isDisabled', () => {
const { container, queryByTestSubject, queryByTitle } = render(
<EuiComboBox
Expand Down

This file was deleted.

1 change: 0 additions & 1 deletion src/components/combo_box/combo_box_input/_index.scss
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
@import 'combo_box_pill';
@import 'combo_box_placeholder';
21 changes: 7 additions & 14 deletions src/components/combo_box/combo_box_input/combo_box_input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -275,18 +275,8 @@ export class EuiComboBoxInput<T> extends Component<
);
}

let placeholderMessage;

if (
placeholder &&
selectedOptions &&
!selectedOptions.length &&
!searchValue
) {
placeholderMessage = (
<p className="euiComboBoxPlaceholder">{placeholder}</p>
);
}
const showPlaceholder =
placeholder && !selectedOptions?.length && !searchValue;

const clickProps: EuiFormControlLayoutIconsProps = {};
if (!isDisabled && onClear && hasSelectedOptions) {
Expand Down Expand Up @@ -344,7 +334,6 @@ export class EuiComboBoxInput<T> extends Component<
tabIndex={-1} // becomes onBlur event's relatedTarget, otherwise relatedTarget is null when clicking on this div
>
{this.renderPills()}
{placeholderMessage}
<EuiComboBoxOptionAppendPrepend
option={this.asPlainText ? selectedOptions?.[0] : undefined}
classNamePrefix="euiComboBoxPlainTextSelection"
Expand All @@ -368,8 +357,12 @@ export class EuiComboBoxInput<T> extends Component<
ref={this.inputRefCallback}
role="combobox"
style={{
inlineSize: this.asPlainText ? '100%' : this.state.inputWidth,
inlineSize:
this.asPlainText || showPlaceholder
? '100%'
: this.state.inputWidth,
}}
placeholder={showPlaceholder ? placeholder : undefined}
value={this.searchValue}
autoFocus={autoFocus}
// Force the menu to re-open on every input click - only necessary when plain text
Expand Down

0 comments on commit 32667ac

Please sign in to comment.