Skip to content

fix(collapse): don't make id prop required #4109

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Sep 18, 2019
Merged

fix(collapse): don't make id prop required #4109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4f2ec40
fix(collapse): don't make `id` prop required
tmorehouse Sep 17, 2019
d40d3d9
Update collapse.js
tmorehouse Sep 17, 2019
983a62d
Update README.md
tmorehouse Sep 17, 2019
d3bd922
Update README.md
jacobmllr95 Sep 18, 2019
470633c
Merge branch 'dev' into tmorehouse/collapse-id
tmorehouse Sep 18, 2019
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
8 changes: 5 additions & 3 deletions src/components/collapse/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ the `visible` prop.

Note, when using `v-model` to control `<b-collapse>`, the `aria-*` attributes and class `collapsed`
are not automatically placed on the trigger button (as is the case when using the `v-b-toggle`
directive). In this example we must control them ourselves.
directive). In this example we **must control the attributes ourselves** for proper accessibility
support.

```html
<template>
Expand Down Expand Up @@ -120,7 +121,8 @@ multiple target IDs using modifiers:
## Accordion support

Turn a group of `<b-collapse>` components into an accordion by supplying an accordion group
identifier via the `accordion` prop:
identifier via the `accordion` prop. Note that only one collapse in an accordion group can be open
at a time.

```html
<template>
Expand Down Expand Up @@ -192,7 +194,7 @@ identifier via the `accordion` prop:
- If using the `v-model` feature of `<b-collapse>` in accordion mode, do not bind the `v-model` or
`visible` prop of all the collapses in the accordion group to the same variable!
- Ensure, at most, only one `<b-collapse>` in the accordion group has the `visible` prop and/or
`v-model` set to `true`.
`v-model` set to `true`. Only one collapse in an accordion group can be open at a time.

## Hiding and showing content in the toggle button based on collapse state

Expand Down
21 changes: 9 additions & 12 deletions src/components/collapse/collapse.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import Vue from '../../utils/vue'
import idMixin from '../../mixins/id'
import listenOnRootMixin from '../../mixins/listen-on-root'
import normalizeSlotMixin from '../../mixins/normalize-slot'
import { isBrowser } from '../../utils/env'
Expand Down Expand Up @@ -32,16 +33,12 @@ const EventOptions = { passive: true, capture: false }
// @vue/component
export const BCollapse = /*#__PURE__*/ Vue.extend({
name: 'BCollapse',
mixins: [listenOnRootMixin, normalizeSlotMixin],
mixins: [idMixin, listenOnRootMixin, normalizeSlotMixin],
model: {
prop: 'visible',
event: 'input'
},
props: {
id: {
type: String,
required: true
},
isNav: {
type: Boolean,
default: false
Expand Down Expand Up @@ -105,7 +102,7 @@ export const BCollapse = /*#__PURE__*/ Vue.extend({
})
// Listen for "Sync state" requests from `v-b-toggle`
this.listenOnRoot(EVENT_STATE_REQUEST, id => {
if (id === this.id) {
if (id === this.safeId()) {
this.$nextTick(this.emitSync)
}
})
Expand Down Expand Up @@ -174,17 +171,17 @@ export const BCollapse = /*#__PURE__*/ Vue.extend({
emitState() {
this.$emit('input', this.show)
// Let v-b-toggle know the state of this collapse
this.$root.$emit(EVENT_STATE, this.id, this.show)
this.$root.$emit(EVENT_STATE, this.safeId(), this.show)
if (this.accordion && this.show) {
// Tell the other collapses in this accordion to close
this.$root.$emit(EVENT_ACCORDION, this.id, this.accordion)
this.$root.$emit(EVENT_ACCORDION, this.safeId(), this.accordion)
}
},
emitSync() {
// Emit a private event every time this component updates to ensure
// the toggle button is in sync with the collapse's state
// It is emitted regardless if the visible state changes
this.$root.$emit(EVENT_STATE_SYNC, this.id, this.show)
this.$root.$emit(EVENT_STATE_SYNC, this.safeId(), this.show)
},
checkDisplayBlock() {
// Check to see if the collapse has `display: block !important;` set.
Expand All @@ -211,7 +208,7 @@ export const BCollapse = /*#__PURE__*/ Vue.extend({
}
},
handleToggleEvt(target) {
if (target !== this.id) {
if (target !== this.safeId()) {
return
}
this.toggle()
Expand All @@ -220,7 +217,7 @@ export const BCollapse = /*#__PURE__*/ Vue.extend({
if (!this.accordion || accordion !== this.accordion) {
return
}
if (openedId === this.id) {
if (openedId === this.safeId()) {
// Open this collapse if not shown
if (!this.show) {
this.toggle()
Expand All @@ -243,7 +240,7 @@ export const BCollapse = /*#__PURE__*/ Vue.extend({
{
class: this.classObject,
directives: [{ name: 'show', value: this.show }],
attrs: { id: this.id || null },
attrs: { id: this.safeId() },
on: { click: this.clickHandler }
},
[this.normalizeSlot('default')]
Expand Down