Skip to content

Commit

Permalink
Merge pull request #5606 from nextcloud-libraries/feat/backport-app-s…
Browse files Browse the repository at this point in the history
…idebar

[next]  feat(NcAppSidebar): Allow to set open state to prevent focus trap issues on mobile #5584
  • Loading branch information
susnux authored May 17, 2024
2 parents 1f36a3b + f3356bc commit 7daf589
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 40 deletions.
3 changes: 3 additions & 0 deletions l10n/messages.pot
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,9 @@ msgstr ""
msgid "Open navigation"
msgstr ""

msgid "Open sidebar"
msgstr ""

msgid "Options"
msgstr ""

Expand Down
193 changes: 153 additions & 40 deletions src/components/NcAppSidebar/NcAppSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ This component provides a way to include the standardised sidebar.
The standard properties like name, subname, starred, etc. allow to automatically
include a standard-header like it's used by the files app.

To conditionally show the sidebar either use `v-if` on the sidebar component,
or use the `open` property of the component to controll the state.
Using `v-show` directly will result in usability issues due to internal focus trap handling.

### Standard usage

```vue
Expand Down Expand Up @@ -373,16 +377,98 @@ A working alternative would be using an icon together with an `aria-label`:
}
</script>
```

### Conditionally show the sidebar

If the sidebar should be shown conditionally (e.g. using a button)
and the users are expected to open and close the sidebar multiple times,
then using `v-if` might result in bad performance.
So instead use the `open` property:

```vue
<template>
<!-- This is in most cases NcContent -->
<div class="content-wrapper">
<!-- The main content - In most cases NcAppContent -->
<div>
<NcButton @click.prevent="showSidebar = !showSidebar">
Toggle sidebar
</NcButton>
</div>
<!-- The sidebar -->
<NcAppSidebar
v-model:open="showSidebar"
name="cat-picture.jpg"
subname="last edited 3 weeks ago">
<NcAppSidebarTab name="Settings" id="settings-tab">
<template #icon>
<Cog :size="20" />
</template>
Single tab content
</NcAppSidebarTab>
</NcAppSidebar>
</div>
</template>
<script>
import Cog from 'vue-material-design-icons/Cog'
export default {
components: {
Cog,
},
data() {
return {
showSidebar: true,
}
},
}
</script>
<style scoped>
/* This styles just mock NcContent and NcAppContent */
.content-wrapper {
position: relative;
/* Just to prevent jumping when the sidebar is hidden */
min-height: 360px;
}
.main-content {
position: absolute;
height: 100%;
width: 100%;
}
/* Fix styles on this style guide page */
@media only screen and (max-width: 512px) {
:deep(aside) {
width: calc(100vw - 64px) !important;
}
}
</style>
```
</docs>

<template>
<NcButton v-if="!open"
:aria-label="t('Open sidebar')"
class="app-sidebar__toggle"
:class="toggleClasses"
type="tertiary"
ref="toggle"
@click="$emit('update:open', true)">
<template #icon>
<!-- @slot Custom icon for the toggle button, defaults to the dock-right icon from MDI -->
<slot name="toggle-icon">
<IconDockRight :size="20" />
</slot>
</template>
</NcButton>
<transition appear
name="slide-right"
@before-enter="onBeforeEnter"
v-bind="$attrs"
@after-enter="onAfterEnter"
@before-leave="onBeforeLeave"
@after-leave="onAfterLeave">
<aside id="app-sidebar-vue"
<aside v-show="open"
id="app-sidebar-vue"
ref="sidebar"
class="app-sidebar"
:aria-labelledby="`app-sidebar-vue-${uid}__header`"
Expand Down Expand Up @@ -429,8 +515,8 @@ A working alternative would be using an icon together with an `aria-label`:
@click.prevent="toggleStarred">
<template #icon>
<NcLoadingIcon v-if="starLoading" />
<Star v-else-if="isStarred" :size="20" />
<StarOutline v-else :size="20" />
<IconStar v-else-if="isStarred" :size="20" />
<IconStarOutline v-else :size="20" />
</template>
</NcButton>
</slot>
Expand Down Expand Up @@ -467,7 +553,7 @@ A working alternative would be using an icon together with an `aria-label`:
:aria-label="changeNameTranslated"
native-type="submit">
<template #icon>
<ArrowRight :size="20" />
<IconArrowRight :size="20" />
</template>
</NcButton>
</form>
Expand Down Expand Up @@ -499,7 +585,7 @@ A working alternative would be using an icon together with an `aria-label`:
class="app-sidebar__close"
@click.prevent="closeSidebar">
<template #icon>
<Close :size="20" />
<IconClose :size="20" />
</template>
</NcButton>
Expand Down Expand Up @@ -532,17 +618,17 @@ import NcButton from '../NcButton/index.ts'
import NcEmptyContent from '../NcEmptyContent/index.js'
import Focus from '../../directives/Focus/index.js'
import Linkify from '../../directives/Linkify/index.js'
import Tooltip from '../../directives/Tooltip/index.js'
import { useIsSmallMobile } from '../../composables/useIsMobile/index.js'
import GenRandomId from '../../utils/GenRandomId.js'
import { getTrapStack } from '../../utils/focusTrap.js'
import { t } from '../../l10n.js'
import isSlotPopulated from '../../utils/isSlotPopulated.ts'
import ArrowRight from 'vue-material-design-icons/ArrowRight.vue'
import Close from 'vue-material-design-icons/Close.vue'
import Star from 'vue-material-design-icons/Star.vue'
import StarOutline from 'vue-material-design-icons/StarOutline.vue'
import IconArrowRight from 'vue-material-design-icons/ArrowRight.vue'
import IconClose from 'vue-material-design-icons/Close.vue'
import IconDockRight from 'vue-material-design-icons/DockRight.vue'
import IconStar from 'vue-material-design-icons/Star.vue'
import IconStarOutline from 'vue-material-design-icons/StarOutline.vue'
import { vOnClickOutside as ClickOutside } from '@vueuse/components'
import { createFocusTrap } from 'focus-trap'
Expand All @@ -553,22 +639,24 @@ export default {
components: {
NcActions,
NcAppSidebarTabs,
ArrowRight,
NcButton,
NcLoadingIcon,
NcEmptyContent,
Close,
Star,
StarOutline,
IconArrowRight,
IconClose,
IconDockRight,
IconStar,
IconStarOutline,
},
directives: {
focus: Focus,
linkify: Linkify,
ClickOutside,
Tooltip,
},
inheritAttrs: false,
props: {
active: {
type: String,
Expand Down Expand Up @@ -675,19 +763,40 @@ export default {
type: String,
default: '',
},
/**
* Allow to conditionally show the sidebar
* You can also use `v-if` on the sidebar, but using the open prop allow to keep
* the sidebar inside the DOM for performance if it is opened and closed multple times.
*
* When using the `open` property to close the sidebar a built-in toggle button will be shown to reopen it,
* similar to the app navigation.
*/
open: {
type: Boolean,
default: true,
},
/**
* Custom classes to assign to the sidebar toggle button
* If needed this can be used to assign styles to the button using `:deep()` selector.
*/
toggleClasses: {
type: [String, Array, Object],
default: '',
},
},
emits: [
'close',
'closing',
'closed',
'opening',
'opened',
// 'figure-click', not emitted on purpose to make "hasFigureClickListener" work
'update:starred',
'update:nameEditable',
'update:name',
'update:active',
'update:name',
'update:nameEditable',
'update:open',
'update:starred',
'submit-name',
'dismiss-editing',
],
Expand Down Expand Up @@ -727,6 +836,10 @@ export default {
isMobile() {
this.toggleFocusTrap()
},
open() {
this.toggleFocusTrap()
},
},
created() {
Expand All @@ -751,6 +864,8 @@ export default {
methods: {
isSlotPopulated,
t,
preserveElementToReturnFocus() {
// Save the element that had focus before the sidebar was opened to return back on close
if (document.activeElement && document.activeElement !== document.body) {
Expand Down Expand Up @@ -793,7 +908,7 @@ export default {
* Activate focus trap if it is currently needed, otherwise deactivate
*/
toggleFocusTrap() {
if (this.isMobile) {
if (this.open && this.isMobile) {
this.initFocusTrap()
this.focusTrap.activate()
} else {
Expand All @@ -813,14 +928,6 @@ export default {
}
},
onBeforeEnter(element) {
/**
* The sidebar is opening and the transition is in progress
*
* @type {HTMLElement}
*/
this.$emit('opening', element)
},
onAfterEnter(element) {
/**
* The sidebar is opened and the transition is complete
Expand All @@ -829,14 +936,6 @@ export default {
*/
this.$emit('opened', element)
},
onBeforeLeave(element) {
/**
* The sidebar is closing and the transition is in progress
*
* @type {HTMLElement}
*/
this.$emit('closing', element)
},
onAfterLeave(element) {
/**
* The sidebar is closed and the transition is complete
Expand All @@ -862,6 +961,11 @@ export default {
* @type {Event}
*/
this.$emit('close', e)
/**
* Current open state emitted after the transitions are finished
* @type {boolean}
*/
this.$emit('update:open', false)
},
/**
Expand Down Expand Up @@ -912,7 +1016,7 @@ export default {
* @public
*/
focus() {
this.$refs.header.focus()
(this.$refs.header ?? this.$refs.toggle)?.focus()
},
/**
Expand Down Expand Up @@ -1012,6 +1116,15 @@ $top-buttons-spacing: 6px;
border-left: 1px solid var(--color-border);
background: var(--color-main-background);
&__toggle {
--sidebar-toggle-position: #{$app-navigation-padding};
position: absolute !important;
inset-block-start: var(--sidebar-toggle-position);
inset-inline-end: var(--sidebar-toggle-position);
// app-content has z-index 1000 so we need 1001
z-index: 1001;
}
.app-sidebar-header {
> .app-sidebar__close {
position: absolute;
Expand Down

0 comments on commit 7daf589

Please sign in to comment.