Skip to content

Commit

Permalink
fix(ui-time-select): clear input field after setting an empty value
Browse files Browse the repository at this point in the history
  • Loading branch information
ToMESSKa authored and matyasf committed Feb 3, 2025
1 parent a3a75e0 commit 1993282
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 13 deletions.
32 changes: 30 additions & 2 deletions cypress/component/TimeSelect.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ import moment from 'moment-timezone'
import 'cypress-real-events'

import '../support/component'
import { TimeSelect } from '../../packages/ui'
import { DateTime } from '../../packages/ui-i18n'
import { TimeSelect } from '@instructure/ui'
import { DateTime } from '@instructure/ui-i18n'

describe('<TimeSelect/>', () => {
it('should render an input and list', async () => {
Expand Down Expand Up @@ -70,6 +70,34 @@ describe('<TimeSelect/>', () => {
})
})

it('should fire onChange when input field is cleared and blurred and allowClearingSelection is true', async () => {
const onChange = cy.spy()
cy.mount(
<TimeSelect
renderLabel="Choose a time"
timezone="US/Eastern"
onChange={onChange}
allowClearingSelection={true}
/>
)
cy.get('input[id^="Select_"]').as('input')

cy.get('@input').click()

cy.get('li[class$="-optionItem"]').eq(0).click()

cy.get('@input').click()
cy.get('@input').clear()
cy.get('@input').blur()

cy.wrap(onChange)
.should('have.been.called')
.then((spy) => {
expect(spy.lastCall.args[1]).to.have.property('value', '')
expect(spy.lastCall.args[1]).to.have.property('inputText', '')
})
})

it('should behave uncontrolled', async () => {
const onChange = cy.spy()
cy.mount(<TimeSelect renderLabel="Choose a time" onChange={onChange} />)
Expand Down
1 change: 1 addition & 0 deletions docs/contributor-docs/v11-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,4 @@ isWIP: true
## Changes

- `ui-dom-utils`/`getComputedStyle` can now return `undefined`: In previous versions sometimes returned an empty object which could lead to runtime exceptions when one tried to call methods of `CSSStyleDeclaration` on it.
- TO-DO: [TimeSelect](/#TimeSelect) as a controlled component can now return an '' instead of a valid date when its input field is cleared. In previous versions it always reverted to the last selected value when the input field was cleared and lost focus.
40 changes: 31 additions & 9 deletions packages/ui-time-select/src/TimeSelect/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class TimeSelect extends Component<TimeSelectProps, TimeSelectState> {
placement: 'bottom stretch',
constrain: 'window',
renderEmptyOption: '---',
allowNonStepInput: false
allowNonStepInput: false,
allowClearingSelection: false
}
static contextType = ApplyLocaleContext

Expand Down Expand Up @@ -220,7 +221,8 @@ class TimeSelect extends Component<TimeSelectProps, TimeSelectState> {
: initialOptions,
isShowingOptions: false,
highlightedOptionId: initialSelection ? initialSelection.id : undefined,
selectedOptionId: initialSelection ? initialSelection.id : undefined
selectedOptionId: initialSelection ? initialSelection.id : undefined,
isInputCleared: false
}
}

Expand Down Expand Up @@ -338,7 +340,8 @@ class TimeSelect extends Component<TimeSelectProps, TimeSelectState> {
selectedOptionId: this.isControlled
? this.state.selectedOptionId
: undefined,
fireChangeOnBlur: undefined
fireChangeOnBlur: undefined,
isInputCleared: this.props.allowClearingSelection && value === ''
})
}
this.setState({
Expand Down Expand Up @@ -386,7 +389,7 @@ class TimeSelect extends Component<TimeSelectProps, TimeSelectState> {
// when pressing ESC. NOT called when an item is selected via Enter/click,
// (but in this case it will be called later when the input is blurred.)
handleBlurOrEsc: SelectProps['onRequestHideOptions'] = (event) => {
const { selectedOptionId, inputValue } = this.state
const { selectedOptionId, inputValue, isInputCleared } = this.state
let defaultValue = ''
if (this.props.defaultValue) {
const date = DateTime.parse(
Expand All @@ -400,14 +403,23 @@ class TimeSelect extends Component<TimeSelectProps, TimeSelectState> {
}
const selectedOption = this.getOption('id', selectedOptionId)
let newInputValue = defaultValue
if (selectedOption) {
// If there is a selected option use its value in the input field.
newInputValue = selectedOption.label
}
// if input was completely cleared, ensure it stays clear
// e.g. defaultValue defined, but no selection yet made
else if (inputValue === '') {
if (inputValue === '' && this.props.allowClearingSelection) {
newInputValue = ''
} else if (selectedOption) {
// If there is a selected option use its value in the input field.
newInputValue = selectedOption.label
} else if (this.props.value) {
// If controlled and input is cleared and blurred after the first render, it should revert to value
const date = DateTime.parse(
this.props.value,
this.locale(),
this.timezone()
)
newInputValue = this.props.format
? date.format(this.props.format)
: date.toISOString()
}
this.setState(() => ({
isShowingOptions: false,
Expand All @@ -421,6 +433,16 @@ class TimeSelect extends Component<TimeSelectProps, TimeSelectState> {
value: this.state.fireChangeOnBlur.value.toISOString(),
inputText: this.state.fireChangeOnBlur.label
})
} else if (
isInputCleared &&
(event as any).key !== 'Escape' &&
this.props.allowClearingSelection
) {
this.setState(() => ({ isInputCleared: false }))
this.props.onChange?.(event, {
value: '',
inputText: ''
})
}
// TODO only fire this if handleSelectOption was not called before.
this.props.onHideOptions?.(event)
Expand Down
15 changes: 13 additions & 2 deletions packages/ui-time-select/src/TimeSelect/props.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,11 @@ type TimeSelectOwnProps = {
*/
valueAsISOString?: string
) => void
/**
* Whether to allow for the user to clear the selected option in the input field.
* If `false`, the input field will return the last selected option after the input is cleared and loses focus.
*/
allowClearingSelection: boolean
}

const propTypes: PropValidators<PropKeys> = {
Expand Down Expand Up @@ -278,7 +283,8 @@ const propTypes: PropValidators<PropKeys> = {
locale: PropTypes.string,
timezone: PropTypes.string,
allowNonStepInput: PropTypes.bool,
onInputChange: PropTypes.func
onInputChange: PropTypes.func,
allowClearingSelection: PropTypes.bool
}

const allowedProps: AllowedPropKeys = [
Expand Down Expand Up @@ -313,7 +319,8 @@ const allowedProps: AllowedPropKeys = [
'locale',
'timezone',
'allowNonStepInput',
'onInputChange'
'onInputChange',
'allowClearingSelection'
]

type TimeSelectOptions = {
Expand Down Expand Up @@ -356,6 +363,10 @@ type TimeSelectState = {
* fire onChange event when the popup closes?
*/
fireChangeOnBlur?: TimeSelectOptions
/**
* Whether to selected option is cleared
*/
isInputCleared: boolean
}

export type { TimeSelectProps, TimeSelectState, TimeSelectOptions }
Expand Down

0 comments on commit 1993282

Please sign in to comment.