Skip to content

Commit

Permalink
fix(VSelect): resolve bug in safari/ie11 with event order disparity
Browse files Browse the repository at this point in the history
the ordering of events in safari/ie11 caused our manual blur of the input to break tabbing through
inputs. also resolved duplicate blur events.

fixes #6608, fixes #5913
  • Loading branch information
johnleider committed Jun 19, 2019
1 parent 4963f72 commit 5fa6a68
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 11 deletions.
9 changes: 2 additions & 7 deletions packages/vuetify/src/components/VSelect/VSelect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,12 +287,10 @@ export default baseMixins.extend<options>().extend({

methods: {
/** @public */
blur (e: Event) {
blur (e?: Event) {
VTextField.options.methods.blur.call(this, e)
this.isMenuActive = false
this.isFocused = false
this.$refs.input && this.$refs.input.blur()
this.selectedIndex = -1
this.onBlur(e)
},
/** @public */
activateMenu () {
Expand Down Expand Up @@ -538,9 +536,6 @@ export default baseMixins.extend<options>().extend({
getValue (item: object) {
return getPropertyFromItem(item, this.itemValue, this.getText(item))
},
onBlur (e: Event) {
this.$emit('blur', e)
},
onChipInput (item: object) {
if (this.multiple) this.selectItem(item)
else this.setValue(null)
Expand Down
11 changes: 8 additions & 3 deletions packages/vuetify/src/components/VTextField/VTextField.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,13 @@ export default baseMixins.extend<options>().extend({
this.onFocus()
},
/** @public */
blur () {
this.$refs.input ? this.$refs.input.blur() : this.onBlur()
blur (e?: Event) {
// https://github.com/vuetifyjs/vuetify/issues/5913
// Safari tab order gets broken if called synchronous
window.requestAnimationFrame(() => {
this.$refs.input && this.$refs.input.blur()
})
this.onBlur(e)
},
clearableCallback () {
this.internalValue = null
Expand Down Expand Up @@ -397,7 +402,7 @@ export default baseMixins.extend<options>().extend({
},
onBlur (e?: Event) {
this.isFocused = false
this.$emit('blur', e)
e && this.$emit('blur', e)
},
onClick () {
if (this.isFocused || this.disabled) return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ describe('VTextField.ts', () => { // eslint-disable-line max-statements
expect(change).toHaveBeenCalledTimes(2)
})

it('should have focus and blur methods', () => {
it('should have focus and blur methods', async () => {
const wrapper = mountFunction()
const focus = jest.fn()
const blur = jest.fn()
Expand All @@ -616,6 +616,12 @@ describe('VTextField.ts', () => { // eslint-disable-line max-statements
expect(focus).toHaveBeenCalledTimes(1)

wrapper.vm.blur()

// https://github.com/vuetifyjs/vuetify/issues/5913
// Blur waits a requestAnimationFrame
// to resolve a bug in MAC / Safari
await new Promise(resolve => window.requestAnimationFrame(resolve))

expect(blur).toHaveBeenCalledTimes(1)
})

Expand Down

11 comments on commit 5fa6a68

@victorwpbastos
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be included in version 1 too?

@johnleider
Copy link
Member Author

Choose a reason for hiding this comment

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

Should be able to. I'll mark this as a post 2.0 release resolution for v1.5

@victorwpbastos
Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea on when this patch will be applied to v1.5?

@johnleider
Copy link
Member Author

Choose a reason for hiding this comment

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

v1.5 patches will start next week.

@norman-appel
Copy link

Choose a reason for hiding this comment

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

can you give us an update if it comes with a patch for v.1.5 ?

@userquin
Copy link
Member

Choose a reason for hiding this comment

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

@johnleider, @KaelWD: please, can anyone confirm (or NOT) if this PR will be included on v1.5 branch?

The bug was opened on Feb. 26, PR to fix it on Jun. 19 (v2 branch), v1.5 patches starts on Jul. 24 and we are on Sep. 4.

I dont want to wait another 6 months ;).

@johnleider
Copy link
Member Author

Choose a reason for hiding this comment

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

Yea. I'll add it for next week's release.

@Keithcat767
Copy link

Choose a reason for hiding this comment

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

@johnleider Did this end up getting released for v1.5? I am still seeing the behavior as of v1.5.19 and couldn't find a release with this commit in the notes. Sorry if I am just missing something. Thanks!

@johnleider
Copy link
Member Author

@johnleider johnleider commented on 5fa6a68 Oct 15, 2019 via email

Choose a reason for hiding this comment

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

@victorwpbastos
Copy link
Contributor

Choose a reason for hiding this comment

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

Any progress on this being included in v1 branch?

@maxshuty
Copy link

Choose a reason for hiding this comment

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

Any progress on this being included in v1 branch?

@victorwpbastos I put up the PR and it is now included in v1.5.23

Please sign in to comment.