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

fix(select components): blur when selecting an option #1419

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
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
4 changes: 2 additions & 2 deletions collections/forms/i18n/en.pot
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"Content-Transfer-Encoding: 8bit\n"
"Plural-Forms: nplurals=2; plural=(n != 1)\n"
"POT-Creation-Date: 2023-05-05T12:46:11.864Z\n"
"PO-Revision-Date: 2023-05-05T12:46:11.864Z\n"
"POT-Creation-Date: 2023-11-08T11:16:26.085Z\n"
"PO-Revision-Date: 2023-11-08T11:16:26.085Z\n"

msgid "Upload file"
msgstr "Upload file"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ Given('the MultiSelect is closed', () => {
})

Given('the MultiSelect is focused', () => {
cy.get('[data-test="dhis2-uicore-select-input"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select-input')
cy.get('[data-test="dhis2-uicore-select"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select')
})

When('the MultiSelect input is clicked', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ Given('the MultiSelect is closed', () => {
})

Given('the MultiSelect is focused', () => {
cy.get('[data-test="dhis2-uicore-select-input"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select-input')
cy.get('[data-test="dhis2-uicore-select"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select')
})

When('the down arrowkey is pressed on the focused element', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,5 +45,5 @@ Then('the options are displayed', () => {
})

Then('the MultiSelect has focus', () => {
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select-input')
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select')
})
9 changes: 1 addition & 8 deletions components/select/src/select/input-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ const InputWrapper = ({
dataTest,
onToggle,
children,
tabIndex,
error,
warning,
valid,
Expand All @@ -29,7 +28,6 @@ const InputWrapper = ({
<div
className={classNames}
onClick={onToggle}
tabIndex={tabIndex}
ref={inputRef}
data-test={dataTest}
>
Expand All @@ -52,7 +50,7 @@ const InputWrapper = ({
box-shadow: inset 0 0 1px 0 rgba(48, 54, 60, 0.1);
}

.root:not(.disabled):focus,
:global(:focus) > .root:not(.disabled),
.root:not(.disabled):active {
outline: none;
box-shadow: inset 0 0 0 2px ${theme.focus};
Expand Down Expand Up @@ -97,14 +95,9 @@ const InputWrapper = ({
)
}

InputWrapper.defaultProps = {
tabIndex: '0',
}

InputWrapper.propTypes = {
dataTest: PropTypes.string.isRequired,
inputRef: PropTypes.object.isRequired,
tabIndex: PropTypes.string.isRequired,
onToggle: PropTypes.func.isRequired,
children: PropTypes.element,
className: PropTypes.string,
Expand Down
28 changes: 18 additions & 10 deletions components/select/src/select/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class Select extends Component {

componentDidMount() {
if (this.props.initialFocus) {
this.inputRef.current.focus()
this.selectRef.current.focus()
}

this.setState({ menuWidth: this.getMenuWidth() })
Expand Down Expand Up @@ -95,18 +95,12 @@ export class Select extends Component {
this.state.open ? this.handleClose() : this.handleOpen()
}

onOutsideClick = (e) => {
const { onBlur, disabled, selected } = this.props

if (disabled) {
onOutsideClick = () => {
if (this.props.disabled) {
return
}

this.handleClose()

if (onBlur) {
onBlur({ selected }, e)
}
Comment on lines -106 to -109
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would cause a second call to onBlur

}

onKeyDown = (e) => {
Expand Down Expand Up @@ -182,16 +176,23 @@ export class Select extends Component {

return (
<div
tabIndex={disabled ? '-1' : tabIndex}
className={className}
ref={this.selectRef}
onFocus={this.onFocus}
onBlur={(e) => {
if (!this.props.onBlur) {
return
}

this.props.onBlur({ selected }, e)
}}
onKeyDown={this.onKeyDown}
data-test={dataTest}
>
<InputWrapper
onToggle={this.onToggle}
inputRef={this.inputRef}
tabIndex={disabled ? '-1' : tabIndex}
error={error}
warning={warning}
valid={valid}
Expand All @@ -212,13 +213,20 @@ export class Select extends Component {
{menu}
</MenuWrapper>
)}

<style jsx>{`
div:focus {
outline: 0;
}
`}</style>
</div>
)
}
}

Select.defaultProps = {
dataTest: 'dhis2-uicore-select',
tabIndex: '0',
}

Select.propTypes = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,10 @@ Feature: Calls onBlur cb when blurred
And the SingleSelect has focus
When the user clicks the backdrop layer
Then the onBlur handler is called

Scenario: Blurring a SingleSelect by interacting selecting an option and interacting with other input
Given a SingleSelect with onBlur handler is rendered
And the SingleSelect input is clicked
And the SingleSelect has focus
When the user selects the first option
Then the onBlur handler is called
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
import '../common/index.js'
import { Given, Then } from 'cypress-cucumber-preprocessor/steps'
import { Given, When, Then, After } from 'cypress-cucumber-preprocessor/steps'

After(() => {
cy.window().then((win) => {
win.onChange.reset()
win.onFocus.reset()
win.onBlur.reset()
})
})

Given('a SingleSelect with onBlur handler is rendered', () => {
cy.visitStory('SingleSelect', 'With onBlur')
})

When('the user selects the first option', () => {
cy.get('[data-test="first-option"]').click()
})

Then('the onBlur handler is called', () => {
cy.window().should((win) => {
expect(win.onBlur).to.be.calledOnce
expect(win.onBlur).to.be.calledWith({
selected: '',
})
const onBlur = win.onBlur
expect(onBlur).to.be.calledOnce
expect(onBlur.firstCall.args[0]).to.eql({ selected: '' })
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ Given('the SingleSelect is closed', () => {
})

Given('the SingleSelect is focused', () => {
cy.get('[data-test="dhis2-uicore-select-input"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select-input')
cy.get('[data-test="dhis2-uicore-select"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select')
})

When('the down arrowkey is pressed on the focused element', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ Given('the SingleSelect is closed', () => {
})

Given('the SingleSelect is focused', () => {
cy.get('[data-test="dhis2-uicore-select-input"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select-input')
cy.get('[data-test="dhis2-uicore-select"]').focus()
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select')
})

When('the down arrowkey is pressed on the focused element', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Given('the SingleSelect is open', () => {
})

When('the SingleSelect input is clicked', () => {
cy.get('[data-test="dhis2-uicore-select-input"]').click()
cy.get('[data-test="dhis2-uicore-select"]').click()
})

When('the user clicks the backdrop layer', () => {
Expand All @@ -40,5 +40,5 @@ Then('the options are displayed', () => {
})

Then('the SingleSelect has focus', () => {
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select-input')
cy.focused().should('have.attr', 'data-test', 'dhis2-uicore-select')
})
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,22 @@ storiesOf('SingleSelect', module)
</SingleSelect>
))
.add('With onBlur', () => (
<SingleSelect className="select" onBlur={window.onBlur}>
<SingleSelectOption value="1" label="option one" />
<SingleSelectOption value="2" label="option two" />
<SingleSelectOption value="3" label="option three" />
</SingleSelect>
<>
<SingleSelect
className="select"
onChange={window.onChange}
onBlur={window.onBlur}
>
<SingleSelectOption
dataTest="first-option"
value="1"
label="option one"
/>
<SingleSelectOption value="2" label="option two" />
<SingleSelectOption value="3" label="option three" />
</SingleSelect>
<input type="input" className="input" />
</>
))
.add('With custom options and onChange', () => (
<SingleSelect className="select" onChange={window.onChange}>
Expand Down
Loading
Loading