Skip to content
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.

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

[next] fix(NcAppSidebar): make sidebar a single node again to allow v-show, classes and attributes #5725

Merged
merged 1 commit into from
Jun 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
162 changes: 136 additions & 26 deletions src/components/NcAppSidebar/NcAppSidebar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -361,24 +361,25 @@
</script>
```

### Conditionally show the sidebar
### Conditionally show the sidebar with `open`

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.
If the sidebar should be shown conditionally, you can use `open` prop to define sidebar visibility.
It automatically shows a toggle button to open the sidebar if it is closed.

You can also use `--app-sidebar-offset` CSS variable to preserve space for the toggle button, for example, in top bar of NcAppContent.
You can also use `--app-sidebar-offset` CSS variable to preserve space
for the toggle button, for example, in top bar of `NcAppContent`.

The built-in toggle button can be removed with `no-toggle` prop.

Note: the built-in toggle button is only available then NcAppSidebar is used in NcContent.

```vue
<template>
<!-- This is in most cases NcContent -->
<NcContent app-name="styleguidist" class="content-styleguidist">
<NcAppContent>
<div class="top-bar">
<NcButton @click.prevent="showSidebar = !showSidebar">
Toggle sidebar
</NcButton>
<NcButton type="primary">Start a call</NcButton>
</div>
</NcAppContent>
<!-- The sidebar -->
Expand Down Expand Up @@ -431,6 +432,87 @@
}
}

.top-bar {
display: flex;
justify-content: flex-end;
/* preserve space for toggle button */
padding-inline-end: var(--app-sidebar-offset);
/* same as on toggle button, but doesn't have to be the same */
margin: var(--app-sidebar-padding);
}
</style>
```

### Conditionally show the sidebar programmatically with `v-if`

If the sidebar should be shown conditionally without any explicit toggle button, you can use `v-if`.

**Note about performance**: using `v-if` might result in bad performance and loosing sidebar content state.

**Note about `v-show`**: using `v-show` to hide sidebar will result in usability issues due to active focus trap on mobile.

```vue
<template>
<!-- This is in most cases NcContent -->
<NcContent app-name="styleguidist" class="content-styleguidist">
<NcAppContent>
<div class="top-bar">
<NcButton @click.prevent="showSidebar = true">
Toggle sidebar
</NcButton>
</div>
</NcAppContent>
<!-- The sidebar -->
<NcAppSidebar
v-if="showSidebar"
name="cat-picture.jpg"
subname="last edited 3 weeks ago"
@close="showSidebar = false">
<NcAppSidebarTab name="Settings" id="settings-tab">
<template #icon>
<Cog :size="20" />
</template>
Single tab content
</NcAppSidebarTab>
</NcAppSidebar>
</NcContent>
</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-styleguidist {
position: relative !important;
/* 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;
}
}

.top-bar {
display: flex;
justify-content: flex-end;
Expand All @@ -444,24 +526,8 @@
</docs>

<template>
<NcButton v-if="!open && !noToggle"
ref="toggle"
:aria-label="t('Open sidebar')"
class="app-sidebar__toggle"
:class="toggleClasses"
type="tertiary"
v-bind="toggleAttrs"
@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"
v-bind="$attrs"
@after-enter="onAfterEnter"
@after-leave="onAfterLeave">
<aside v-show="open"
Expand All @@ -470,6 +536,28 @@
class="app-sidebar"
:aria-labelledby="`app-sidebar-vue-${uid}__header`"
@keydown.esc="onKeydownEsc">
<!--
We cannot render toggle button inside sidebar (aside#app-sidebar-vue), because it is hidden then the toggle is needed.
But we also need transition with the sidebar to be the root of this component to use it as a single UI element, allowing to use `v-show`.
So we cannot render the toggle button directly in this component.
As a simple solution - render it in the content to keep correct position.
-->
<Teleport v-if="ncContentSelector && !open && !noToggle" :to="ncContentSelector">
<NcButton :aria-label="t('Open sidebar')"
class="app-sidebar__toggle"
:class="toggleClasses"
type="tertiary"
v-bind="toggleAttrs"
@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>
</Teleport>

<header :class="{
'app-sidebar-header--with-figure': isSlotPopulated($slots.header?.()) || background,
'app-sidebar-header--compact': compact,
Expand Down Expand Up @@ -652,7 +740,12 @@
ClickOutside,
},

inheritAttrs: false,
inject: {
ncContentSelector: {
from: 'NcContent:selector',
default: undefined,
},
},

props: {
active: {
Expand Down Expand Up @@ -852,11 +945,15 @@

open() {
this.toggleFocusTrap()

this.checkToggleButtonContainerAvailability()
},
},

created() {
this.preserveElementToReturnFocus()

this.checkToggleButtonContainerAvailability()
},

mounted() {
Expand Down Expand Up @@ -990,7 +1087,7 @@
* @type {Event}
*/
// eslint-disable-next-line vue/require-explicit-emits
this.$emit('figure-click', e)

Check warning on line 1090 in src/components/NcAppSidebar/NcAppSidebar.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Custom event name 'figure-click' must be camelCase
},

/**
Expand Down Expand Up @@ -1040,6 +1137,19 @@
this.$refs.tabs.focusActiveTabContent()
},

/**
* Check if the toggle button container is available
*/
checkToggleButtonContainerAvailability() {
// Toggle button must be rendered, but there is no element to teleport it to
if (this.open === false && !this.noToggle && !this.ncContentSelector) {
console.warn(
'[NcAppSidebar] It looks like you want to use NcAppSidebar with the built-in toggle button. '
+ 'This feature is only available when NcAppSidebar is used in NcContent.',
)
}
},

/**
* Emit name change event to parent component
*
Expand Down Expand Up @@ -1068,7 +1178,7 @@
*
* @type {Event}
*/
this.$emit('submit-name', event)

Check warning on line 1181 in src/components/NcAppSidebar/NcAppSidebar.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Custom event name 'submit-name' must be camelCase
},
onDismissEditing() {
// Disable editing
Expand All @@ -1078,7 +1188,7 @@
*
* @type {Event}
*/
this.$emit('dismiss-editing')

Check warning on line 1191 in src/components/NcAppSidebar/NcAppSidebar.vue

View workflow job for this annotation

GitHub Actions / NPM lint

Custom event name 'dismiss-editing' must be camelCase
},
onUpdateActive(activeTab) {
/**
Expand Down
1 change: 1 addition & 0 deletions src/components/NcContent/NcContent.vue
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ export default {
provide() {
return {
'NcContent:setHasAppNavigation': this.setAppNavigation,
'NcContent:selector': '#content-vue',
}
},
props: {
Expand Down
Loading