Skip to content

Commit

Permalink
fix(b-table, b-table-lite): handle edge case with row events when tab…
Browse files Browse the repository at this point in the history 
…le is removed from dom. instantiate row event handlers only when listeners are registered (fixes #4384) (#4388)

* fix(b-table, b-table-lite): handle edge case with row events when table is removed from dom (fixes #4384)

* Update mixin-tbody.js

* Update mixin-tbody-row.js

* Update table-tbody-row-events.spec.js

* Update table-tbody-row-events.spec.js

* Update mixin-tbody-row.js

* only emit events if listeners are registered

* Update table-tbody-row-events.spec.js

* Update mixin-tbody-row.js

* Update mixin-tbody.js

* Update mixin-tbody.js

* Update mixin-tbody-row.js

* Update mixin-tbody.js
  • Loading branch information
@tmorehouse @jacobmllr95
tmorehouse authored and jacobmllr95 committed Nov 15, 2019
1 parent 136a72b commit 9a81cd4
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 23 deletions.
10 changes: 6 additions & 4 deletions src/components/table/helpers/mixin-tbody-row.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,19 @@ export default {
rowHovered(evt) {
// `mouseenter` handler (non-bubbling)
// `this.tbodyRowEvtStopped` from tbody mixin
if (!this.tbodyRowEvtStopped(evt)) {
const type = 'row-hovered'
if (this.$listeners[type] && !this.tbodyRowEvtStopped(evt)) {
// `this.emitTbodyRowEvent` from tbody mixin
this.emitTbodyRowEvent('row-hovered', evt)
this.emitTbodyRowEvent(type, evt)
}
},
rowUnhovered(evt) {
// `mouseleave` handler (non-bubbling)
// `this.tbodyRowEvtStopped` from tbody mixin
if (!this.tbodyRowEvtStopped(evt)) {
const type = 'row-unhovered'
if (this.$listeners[type] && !this.tbodyRowEvtStopped(evt)) {
// `this.emitTbodyRowEvent` from tbody mixin
this.emitTbodyRowEvent('row-unhovered', evt)
this.emitTbodyRowEvent(type, evt)
}
},
// Render helpers
Expand Down
43 changes: 24 additions & 19 deletions src/components/table/helpers/mixin-tbody.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,12 @@ export default {
// `this.$refs.itemRows` is an array of item TR components/elements
// Rows should all be B-TR components, but we map to TR elements
// Also note that `this.$refs.itemRows` may not always be in document order
const tbody = this.$refs.tbody.$el || this.$refs.tbody
const trs = (this.$refs.itemRows || []).map(tr => tr.$el || tr)
// TODO: This may take time for tables many rows, so we may want to cache
// the result of this during each render cycle on a non-reactive
// property. We clear out the cache as each render starts, and
// populate it on first access of this method if null
return arrayFrom(tbody.children).filter(tr => arrayIncludes(trs, tr))
const refs = this.$refs || {}
const tbody = refs.tbody ? refs.tbody.$el || refs.tbody : null
const trs = (refs.itemRows || []).map(tr => tr.$el || tr)
return tbody && tbody.children && tbody.children.length > 0 && trs && trs.length > 0
? arrayFrom(tbody.children).filter(tr => arrayIncludes(trs, tr))
: []
},
getTbodyTrIndex(el) {
// Returns index of a particular TBODY item TR
Expand Down Expand Up @@ -102,6 +101,7 @@ export default {
}
},
onTBodyRowClicked(evt) {
// Row-clicked handler is only added when needed
if (this.tbodyRowEvtStopped(evt)) {
// If table is busy, then don't propagate
return
Expand All @@ -113,18 +113,21 @@ export default {
this.emitTbodyRowEvent('row-clicked', evt)
},
onTbodyRowMiddleMouseRowClicked(evt) {
if (!this.tbodyRowEvtStopped(evt) && evt.which === 2) {
this.emitTbodyRowEvent('row-middle-clicked', evt)
const type = 'row-middle-clicked'
if (this.$listeners[type] && !this.tbodyRowEvtStopped(evt) && evt.which === 2) {
this.emitTbodyRowEvent(type, evt)
}
},
onTbodyRowContextmenu(evt) {
if (!this.tbodyRowEvtStopped(evt)) {
this.emitTbodyRowEvent('row-contextmenu', evt)
const type = 'row-contextmenu'
if (this.$listeners[type] && !this.tbodyRowEvtStopped(evt)) {
this.emitTbodyRowEvent(type, evt)
}
},
onTbodyRowDblClicked(evt) {
if (!this.tbodyRowEvtStopped(evt) && !filterEvent(evt)) {
this.emitTbodyRowEvent('row-dblclicked', evt)
const type = 'row-dblclicked'
if (this.$listeners[type] && !this.tbodyRowEvtStopped(evt) && !filterEvent(evt)) {
this.emitTbodyRowEvent(type, evt)
}
},
// Note: Row hover handlers are handled by the tbody-row mixin
Expand Down Expand Up @@ -187,22 +190,24 @@ export default {
$rows.push(this.renderBottomRow ? this.renderBottomRow() : h())
}

// Note: these events will only emit if a listener is registered
const handlers = {
// TODO: We may want to to only instantiate these handlers
// if there is an event listener registered
auxclick: this.onTbodyRowMiddleMouseRowClicked,
// TODO: Perhaps we do want to automatically prevent the
// default context menu from showing if there is
// a `row-contextmenu` listener registered.
// TODO:
// Perhaps we do want to automatically prevent the
// default context menu from showing if there is a
// `row-contextmenu` listener registered
contextmenu: this.onTbodyRowContextmenu,
// The following event(s) is not considered A11Y friendly
dblclick: this.onTbodyRowDblClicked
// hover events (mouseenter/mouseleave) ad handled by tbody-row mixin
// Hover events (`mouseenter`/`mouseleave`) are handled by `tbody-row` mixin
}
// Add in click/keydown listeners if needed
if (hasRowClickHandler) {
handlers.click = this.onTBodyRowClicked
handlers.keydown = this.onTbodyRowKeydown
}

// Assemble rows into the tbody
const $tbody = h(
BTbody,
Expand Down
74 changes: 74 additions & 0 deletions src/components/table/table-tbody-row-events.spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ describe('table > tbody row events', () => {
propsData: {
fields: testFields,
items: testItems
},
listeners: {
// Row-dblclicked will only occur if there is a registered listener
'row-dblclicked': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -104,6 +108,10 @@ describe('table > tbody row events', () => {
fields: testFields,
items: testItems,
busy: true
},
listeners: {
// Row-dblclicked will only occur if there is a registered listener
'row-dblclicked': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -121,6 +129,10 @@ describe('table > tbody row events', () => {
propsData: {
fields: testFields,
items: testItems
},
listeners: {
// Row-middle-clicked will only occur if there is a registered listener
'row-middle-clicked': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -143,6 +155,10 @@ describe('table > tbody row events', () => {
fields: testFields,
items: testItems,
busy: true
},
listeners: {
// Row-middle-clicked will only occur if there is a registered listener
'row-middle-clicked': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -160,6 +176,10 @@ describe('table > tbody row events', () => {
propsData: {
fields: testFields,
items: testItems
},
listeners: {
// Row-contextmenu will only occur if there is a registered listener
'row-contextmenu': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -182,6 +202,10 @@ describe('table > tbody row events', () => {
fields: testFields,
items: testItems,
busy: true
},
listeners: {
// Row-contextmenu will only occur if there is a registered listener
'row-contextmenu': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -199,6 +223,10 @@ describe('table > tbody row events', () => {
propsData: {
fields: testFields,
items: testItems
},
listeners: {
// Row-hovered will only occur if there is a registered listener
'row-hovered': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -215,12 +243,33 @@ describe('table > tbody row events', () => {
wrapper.destroy()
})

it('should not emit row-hovered event when a row is hovered and no listener', async () => {
const wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems
}
})
expect(wrapper).toBeDefined()
const $rows = wrapper.findAll('tbody > tr')
expect($rows.length).toBe(3)
expect(wrapper.emitted('row-hovered')).not.toBeDefined()
$rows.at(1).trigger('mouseenter')
expect(wrapper.emitted('row-hovered')).not.toBeDefined()

wrapper.destroy()
})

it('should not emit row-hovered event when a row is hovered and table busy', async () => {
const wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems,
busy: true
},
listeners: {
// Row-hovered will only occur if there is a registered listener
'row-hovered': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -238,6 +287,10 @@ describe('table > tbody row events', () => {
propsData: {
fields: testFields,
items: testItems
},
listeners: {
// Row-unhovered will only occur if there is a registered listener
'row-unhovered': () => {}
}
})
expect(wrapper).toBeDefined()
Expand All @@ -254,12 +307,33 @@ describe('table > tbody row events', () => {
wrapper.destroy()
})

it('should not emit row-nhovered event when a row is hovered and no listener', async () => {
const wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems
}
})
expect(wrapper).toBeDefined()
const $rows = wrapper.findAll('tbody > tr')
expect($rows.length).toBe(3)
expect(wrapper.emitted('row-unhovered')).not.toBeDefined()
$rows.at(1).trigger('mouseleave')
expect(wrapper.emitted('row-unhovered')).not.toBeDefined()

wrapper.destroy()
})

it('should not emit row-unhovered event when a row is unhovered and table busy', async () => {
const wrapper = mount(BTable, {
propsData: {
fields: testFields,
items: testItems,
busy: true
},
listeners: {
// Row-unhovered will only occur if there is a registered listener
'row-unhovered': () => {}
}
})
expect(wrapper).toBeDefined()
Expand Down

0 comments on commit 9a81cd4

Please sign in to comment.