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

fix(NcActions): Use full window height #5806

Merged
merged 2 commits into from
Jul 16, 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
66 changes: 48 additions & 18 deletions src/components/NcActions/NcActions.vue
susnux marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -952,11 +952,13 @@
import NcButton from '../NcButton/index.js'
import NcPopover from '../NcPopover/index.js'
import GenRandomId from '../../utils/GenRandomId.js'
import { getTrapStack } from '../../utils/focusTrap.js'

import { t } from '../../l10n.js'
import { getTrapStack } from '../../utils/focusTrap.js'
import { useElementBounding, useWindowSize } from '@vueuse/core'
import Vue, { ref, computed, toRef } from 'vue'

import Vue, { computed } from 'vue'
import DotsHorizontal from 'vue-material-design-icons/DotsHorizontal.vue'
import IconDotsHorizontal from 'vue-material-design-icons/DotsHorizontal.vue'

const focusableSelector = '.focusable'

Expand All @@ -973,7 +975,6 @@

components: {
NcButton,
DotsHorizontal,
NcPopover,
},

Expand Down Expand Up @@ -1130,7 +1131,7 @@
*/
boundariesElement: {
type: Element,
default: () => document.querySelector('body'),
default: () => document.querySelector('#app-content-vue') ?? document.querySelector('body'),
Copy link
Contributor

@Antreesy Antreesy Jul 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, we didn't check, and that's breaking NcActions in the NcAppNavigation and NcAppSidebar.Shouldn't it be #content-vue, as in NcContent?

a b
image image

},

/**
Expand Down Expand Up @@ -1168,11 +1169,38 @@
'click',
],

setup() {
setup(props) {
const randomId = `menu-${GenRandomId()}`
const triggerRandomId = `trigger-${randomId}`

const triggerButton = ref()

const { top, bottom } = useElementBounding(triggerButton)
const { top: boundaryTop, bottom: boundaryBottom } = useElementBounding(toRef(props, 'boundariesElement'))
const { height: windowHeight } = useWindowSize()
const maxMenuHeight = computed(() => Math.max(
// Either expand to the top
Math.min(
// max height is the top position of the trigger minus the header height minus the wedge and the padding
top.value - 84,
// and also limited to the space in the boundary
top.value - boundaryTop.value,
),
// or expand to the bottom
Math.min(
// the max height is the window height minus current position of the trigger minus the wedge and padding
windowHeight.value - bottom.value - 34,
// and limit to the available space in the boundary
boundaryBottom.value - bottom.value,
),
))

return {
triggerButton,
maxMenuHeight,

randomId,
triggerRandomId: `trigger-${randomId}`,
triggerRandomId,
}
},

Expand Down Expand Up @@ -1416,8 +1444,8 @@
this.focusIndex = 0

if (returnFocus) {
// Focus back the menu button
this.$refs.menuButton?.$el.focus()
// Focus back the trigger button
this.$refs.triggerButton?.$el.focus()
}
},

Expand All @@ -1432,30 +1460,32 @@
},

/**
* Hanle resizing the popover to make sure users can discover there is more to scroll
* Handle resizing the popover to make sure users can discover there is more to scroll
*/
resizePopover() {
// Get the inner v-popper element that defines the popover height (from floating-vue)
const inner = this.$refs.menu.closest('.v-popper__inner')
const maxHeight = Number.parseFloat(window.getComputedStyle(inner).maxHeight)
const height = this.$refs.menu.clientHeight

// If the popover height is limited by the max-height (scrollbars shown) limit the height to half of the last element
if (height > maxHeight) {
if (height > this.maxMenuHeight) {
// sum of action heights
let currentHeight = 0
// last action height
let actionHeight = 0
for (const action of this.$refs.menuList.children) {
// If the max height would be overflown by half of the current element,
// then we limit the height to the half of the previous element
if ((currentHeight + action.clientHeight / 2) > maxHeight) {
if ((currentHeight + action.clientHeight / 2) > this.maxMenuHeight) {
inner.style.height = `${currentHeight - actionHeight / 2}px`
break
}
// otherwise sum up the height
actionHeight = action.clientHeight
currentHeight += actionHeight
}
} else {
inner.style.height = 'fit-content'
}
},

Expand All @@ -1467,7 +1497,7 @@
return this.$refs.menu.querySelector('li.active')
},
/**
* @return {NodeListOf<HTMLElement>}

Check warning on line 1500 in src/components/NcActions/NcActions.vue

View workflow job for this annotation

GitHub Actions / NPM lint

The type 'NodeListOf' is undefined
*/
getFocusableMenuItemElements() {
return this.$refs.menu.querySelectorAll(focusableSelector)
Expand Down Expand Up @@ -1836,7 +1866,7 @@
const triggerIcon = this.$slots.icon?.[0] || (
this.defaultIcon
? h('span', { class: ['icon', this.defaultIcon] })
: h('DotsHorizontal', {
: h(IconDotsHorizontal, {
props: {
size: 20,
},
Expand All @@ -1854,7 +1884,7 @@
container: this.container,
popoverBaseClass: 'action-item__popper',
popupRole: this.config.popupRole,
setReturnFocus: this.config.withFocusTrap ? this.$refs.menuButton?.$el : null,
setReturnFocus: this.config.withFocusTrap ? this.$refs.triggerButton?.$el : null,
focusTrap: this.config.withFocusTrap,
},
// For some reason the popover component
Expand Down Expand Up @@ -1883,7 +1913,7 @@
disabled: this.disabled,
},
slot: 'trigger',
ref: 'menuButton',
ref: 'triggerButton',
attrs: {
id: this.triggerRandomId,
'aria-label': this.menuName ? null : this.ariaLabel,
Expand Down Expand Up @@ -2059,12 +2089,12 @@
// the popover__inner for actions only.
.v-popper--theme-dropdown.v-popper__popper.action-item__popper .v-popper__wrapper {
border-radius: var(--border-radius-large);
overflow:hidden;
overflow: hidden;

.v-popper__inner {
border-radius: var(--border-radius-large);
padding: 4px;
max-height: calc(50vh - 16px);
max-height: calc(100vh - var(--header-height));
overflow: auto;
}
}
Expand Down
1 change: 1 addition & 0 deletions styleguide.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ module.exports = async () => {
path.join(__dirname, 'styleguide/global.requires.js'),
path.join(__dirname, 'styleguide/assets/icons.css'),
path.join(__dirname, 'styleguide/assets/additional.css'),
path.join(__dirname, 'styleguide/assets/styleguide.css'),
],
pagePerSection: true,
minimize: true,
Expand Down
13 changes: 13 additions & 0 deletions styleguide/assets/styleguide.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/*!
* SPDX-FileCopyrightText: 2017 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

#rsg-root {
height: 100%;
}

#rsg-root .rsg--root-1 {
height: 100%;
overflow: scroll;
}
Loading