Skip to content

feat(Select): fix space/enter closing typeahead variants incorrectly #4663

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

Merged
redallen merged 3 commits into from
Aug 5, 2020
Merged

feat(Select): fix space/enter closing typeahead variants incorrectly #4663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ef94430
feat(Select): prevent typeahead variants from reaching other variant …
kmcfaul Aug 5, 2020
a1b2b88
feat(Select): fix integration
kmcfaul Aug 5, 2020
05401b4
feat(Select): fix integration ids
kmcfaul Aug 5, 2020
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 packages/react-core/src/components/Select/SelectToggle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ export class SelectToggle extends React.Component<SelectToggleProps> {
}

if (
variant === SelectVariant.typeahead ||
variant === SelectVariant.typeaheadMulti ||
(event.key === KeyTypes.Tab && variant === SelectVariant.checkbox) ||
(event.key === KeyTypes.Tab && !isOpen) ||
(event.key !== KeyTypes.Enter && event.key !== KeyTypes.Space)
Expand Down
10 changes: 5 additions & 5 deletions packages/react-integration/cypress/integration/select.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ describe('Select Test', () => {

it('Verify Single Select', () => {
cy.get('#single-select').click();
cy.get('#pf-random-id-0-2').click();
cy.get('#Miss-2').click();
cy.get('#single-select')
.contains('Miss')
.should('exist');
});

it('Verify Description Select', () => {
cy.get('#single-select-with-descriptions').click();
cy.get('#pf-random-id-21-2').click();
cy.get('#Miss-2').click();
cy.get('#single-select-with-descriptions')
.contains('Miss')
.should('exist');
Expand All @@ -27,7 +27,7 @@ describe('Select Test', () => {

it('Verify Custom Single Select', () => {
cy.get('#custom-select').click();
cy.get('#pf-random-id-1-2').click();
cy.get('#Miss-2').click();
cy.get('#custom-select')
.contains('text-Miss')
.should('exist');
Expand Down Expand Up @@ -56,7 +56,7 @@ describe('Select Test', () => {
find('#typeahead-select-select-typeahead').should('have.value', '');
find('input:nth-child(1)').type('Flo');
find('#typeahead-select-select-typeahead').should('have.value', 'Flo');
find('input:nth-child(1)').trigger('keydown', { keyCode: 13 });
find('#typeahead-select-select-typeahead').trigger('keydown', { keyCode: 13 });
find('#typeahead-select-select-typeahead').should('have.value', 'Florida');
find('button.pf-c-select__toggle-clear:first').click();
find('#typeahead-select-select-typeahead').should('have.value', '');
Expand All @@ -72,7 +72,7 @@ describe('Select Test', () => {
find('#typeahead-select-select-typeahead').should('have.value', '');
find('input:nth-child(1)').type('Unknown');
find('#typeahead-select-select-typeahead').should('have.value', 'Unknown');
find('input:nth-child(1)').trigger('keydown', { keyCode: 13 });
find('#typeahead-select-select-typeahead').type('{esc}');
find('#typeahead-select-select-typeahead').should('have.value', '');
});

Expand Down
45 changes: 27 additions & 18 deletions packages/react-integration/demo-app-ts/src/components/demos/SelectDemo/SelectDemo.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,16 @@ export class SelectDemo extends Component<SelectDemoState> {
];

checkboxOptions = [
<SelectOption key={0} value="Active" inputId="Active" />,
<SelectOption key={0} value="Active" inputId="Active" id="Active" />,
<SelectOption
key={1}
value={{ numericValue: 0, toString: () => 'Cancelled' } as SelectOptionObject}
inputId="Cancelled"
id="Cancelled"
/>,
<SelectOption key={2} value="Paused" inputId="Paused" />,
<SelectOption key={3} value="Warning" inputId="Warning" />,
<SelectOption key={4} value="Restarted" inputId="Restarted" />
<SelectOption key={2} value="Paused" inputId="Paused" id="Paused" />,
<SelectOption key={3} value="Warning" inputId="Warning" id="Warning" />,
<SelectOption key={4} value="Restarted" inputId="Restarted" id="Restarted" />
];

customTypeaheadOptions = [
Expand Down Expand Up @@ -505,6 +506,7 @@ export class SelectDemo extends Component<SelectDemoState> {
>
{this.singleOptions.map((option, index) => (
<SelectOption
id={option.value}
isDisabled={option.disabled}
key={index}
value={option.value}
Expand Down Expand Up @@ -551,6 +553,7 @@ export class SelectDemo extends Component<SelectDemoState> {
>
{this.singleOptions.map((option, index) => (
<SelectOption
id={option.value}
isDisabled={option.disabled}
key={index}
value={option.value}
Expand Down Expand Up @@ -588,6 +591,7 @@ export class SelectDemo extends Component<SelectDemoState> {
>
{this.singleOptions.map((option, index) => (
<SelectOption
id={option.value}
isDisabled={option.disabled}
key={index}
value={option.value}
Expand Down Expand Up @@ -622,32 +626,32 @@ export class SelectDemo extends Component<SelectDemoState> {
isOpen={customSingleisOpen}
aria-labelledby={titleId}
>
<SelectOption key={0} value="Choose..." isPlaceholder>
<SelectOption key={0} value="Choose..." isPlaceholder id="Choose...">
Choose...
</SelectOption>
<SelectOption key={1} value="Mr">
<SelectOption key={1} value="Mr" id="Mr">
<div>div-Mr</div>
<CartArrowDownIcon />
</SelectOption>
<SelectOption key={2} value="Miss">
<SelectOption key={2} value="Miss" id="Miss">
text-Miss
</SelectOption>
<SelectOption key={3} value="Mrs">
<SelectOption key={3} value="Mrs" id="Mrs">
<div>div-Mrs</div>
<CartArrowDownIcon />
</SelectOption>
<SelectOption key={4} value="Ms">
<SelectOption key={4} value="Ms" id="Ms">
<div>
<span>nested-Ms</span>
</div>
</SelectOption>
<SelectOption key={5} value="Dr">
<SelectOption key={5} value="Dr" id="Dr">
<div>one</div>
<div>
two<div>nested-three-dr</div>
</div>
</SelectOption>
<SelectOption key={6} value="Other">
<SelectOption key={6} value="Other" id="Other">
<div>single-Other</div>
</SelectOption>
</Select>
Expand Down Expand Up @@ -752,7 +756,7 @@ export class SelectDemo extends Component<SelectDemoState> {
noResultsFoundText="Item not found"
>
{typeaheadOptions.map((option, index) => (
<SelectOption isDisabled={option.disabled} key={index} value={option.value} />
<SelectOption isDisabled={option.disabled} key={index} value={option.value} id={option.value} />
))}
</Select>
</div>
Expand Down Expand Up @@ -802,7 +806,7 @@ export class SelectDemo extends Component<SelectDemoState> {
placeholderText="Select a state"
>
{this.state.typeaheadOptions.map((option, index) => (
<SelectOption isDisabled={option.disabled} key={index} value={option.value} />
<SelectOption isDisabled={option.disabled} key={index} value={option.value} id={option.value} />
))}
</Select>
</div>
Expand Down Expand Up @@ -836,7 +840,12 @@ export class SelectDemo extends Component<SelectDemoState> {
placeholderText="Select a state"
>
{this.customTypeaheadOptions.map((option, index) => (
<SelectOption isDisabled={option.disabled} key={index} value={option.value} />
<SelectOption
isDisabled={option.disabled}
key={index}
value={option.value}
id={option.value.toString()}
/>
))}
</Select>
</div>
Expand Down Expand Up @@ -870,7 +879,7 @@ export class SelectDemo extends Component<SelectDemoState> {
placeholderText="Select a state"
>
{this.state.typeaheadOptions.map((option, index) => (
<SelectOption isDisabled={option.disabled} key={index} value={option.value}>
<SelectOption isDisabled={option.disabled} key={index} value={option.value} id={option.value}>
<div>
div-{option.value.toString()}
<span>-test_span</span>
Expand Down Expand Up @@ -911,7 +920,7 @@ export class SelectDemo extends Component<SelectDemoState> {
placeholderText="Select a state"
>
{this.state.typeaheadOptions.map((option, index) => (
<SelectOption isDisabled={option.disabled} key={index} value={option.value}>
<SelectOption isDisabled={option.disabled} key={index} value={option.value} id={option.value}>
<div>
div-{option.value.toString()}
<span>-test_span</span>
Expand Down Expand Up @@ -981,8 +990,8 @@ export class SelectDemo extends Component<SelectDemoState> {
aria-labelledby={titleId}
placeholderText="Select a state"
>
<SelectOption value={'option1'} />
<SelectOption value={'option2'} />
<SelectOption value={'option1'} id="option1" />
<SelectOption value={'option2'} id="option1" />
</Select>
</Form>
</StackItem>
Expand Down