fix(collapse/toggle): persist toggle state on element and prevent multiple state emits (closes #2923) (#2924)

tmorehouse · web-flow · commit 6f899fcff155 · 2019-03-27T11:12:33.000-03:00
diff --git a/src/components/collapse/collapse.js b/src/components/collapse/collapse.js
@@ -92,10 +92,9 @@ export default {
       this.setWindowEvents(true)
     }
   },
-  updated() {
-    this.$root.$emit(EVENT_STATE, this.id, this.show)
-  },
   beforeDestroy() /* istanbul ignore next */ {
+    // Trigger state emit if needed
+    this.show = false
     if (this.isNav && inBrowser) {
       this.setWindowEvents(false)
     }
diff --git a/src/directives/toggle/toggle.js b/src/directives/toggle/toggle.js
@@ -1,21 +1,38 @@
 import target from '../../utils/target'
-import { setAttr, addClass, removeClass } from '../../utils/dom'
-
-// Are we client side?
-const inBrowser = typeof window !== 'undefined'
+import { setAttr, removeAttr, addClass, removeClass } from '../../utils/dom'
+import { inBrowser } from '../../utils/env'
 
 // target listen types
 const listenTypes = { click: true }
 
 // Property key for handler storage
 const BVT = '__BV_toggle__'
+const BVT_STATE = '__BV_toggle_STATE__'
+const BVT_CONTROLS = '__BV_toggle_CONTROLS__'
 
 // Emitted Control Event for collapse (emitted to collapse)
 const EVENT_TOGGLE = 'bv::toggle::collapse'
 
 // Listen to Event for toggle state update (Emited by collapse)
 const EVENT_STATE = 'bv::collapse::state'
 
+/* istanbul ignore next */
+const handleUpdate = (el, binding, vnode) => {
+  // Ensure the collapse class and aria-* attributes persist
+  // after element is updated (eitehr by parent re-rendering
+  // or changes to this element or it's contents.
+  if (inBrowser) {
+    if (el[BVT_STATE] === true) {
+      addClass(el, 'collapsed')
+      setAttr(el, 'aria-expanded', 'true')
+    } else if (el[BVT_STATE] === false) {
+      removeClass(el, 'collapsed')
+      setAttr(el, 'aria-expanded', 'false')
+    }
+    setAttr(el, 'aria-controls', el[BVT_CONTROLS])
+  }
+}
+
 export default {
   bind(el, binding, vnode) {
     const targets = target(vnode, binding, listenTypes, ({ targets, vnode }) => {
@@ -26,7 +43,10 @@ export default {
 
     if (inBrowser && vnode.context && targets.length > 0) {
       // Add aria attributes to element
-      setAttr(el, 'aria-controls', targets.join(' '))
+      el[BVT_CONTROLS] = targets.join(' ')
+      // state is initialy collapsed until we receive a state event
+      el[BVT_STATE] = false
+      setAttr(el, 'aria-controls', el[BVT_CONTROLS])
       setAttr(el, 'aria-expanded', 'false')
       if (el.tagName !== 'BUTTON') {
         // If element is not a button, we add `role="button"` for accessibility
@@ -39,6 +59,7 @@ export default {
           // Set aria-expanded state
           setAttr(el, 'aria-expanded', state ? 'true' : 'false')
           // Set/Clear 'collapsed' class state
+          el[BVT_STATE] = state
           if (state) {
             removeClass(el, 'collapsed')
           } else {
@@ -51,11 +72,18 @@ export default {
       vnode.context.$root.$on(EVENT_STATE, el[BVT])
     }
   },
+  componentUpdated: handleUpdate,
+  updated: handleUpdate,
   unbind(el, binding, vnode) /* istanbul ignore next */ {
     if (el[BVT]) {
       // Remove our $root listener
       vnode.context.$root.$off(EVENT_STATE, el[BVT])
       el[BVT] = null
+      el[BVT_STATE] = null
+      el[BVT_CONTROLS] = null
+      removeClass(el, 'collapsed')
+      removeAttr(el, 'aria-expanded')
+      removeAttr(el, 'aria-controls')
     }
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -92,10 +92,9 @@ export default {`
`92`	`92`	`this.setWindowEvents(true)`
`93`	`93`	`}`
`94`	`94`	`},`
`95`		`- updated() {`
`96`		`- this.$root.$emit(EVENT_STATE, this.id, this.show)`
`97`		`- },`
`98`	`95`	`beforeDestroy() /* istanbul ignore next */ {`
	`96`	`+ // Trigger state emit if needed`
	`97`	`+ this.show = false`
`99`	`98`	`if (this.isNav && inBrowser) {`
`100`	`99`	`this.setWindowEvents(false)`
`101`	`100`	`}`