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(deps): update dependency vuetify to v3.6.8 #1785

Merged
merged 4 commits into from
Aug 14, 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
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"vue-router": "4.4.0",
"vue-the-mask": "0.11.1",
"vue3-apexcharts": "1.4.1",
"vuetify": "3.5.14",
"vuetify": "3.6.8",
"vuex": "4.1.0"
},
"devDependencies": {
Expand Down
4 changes: 2 additions & 2 deletions src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ const reducedAnimation = useReducedAnimation()

const vuetifyDefaults = computed(() => ({
global: {
transition: reducedAnimation.value ? false : null,
ripple: reducedAnimation.value ? false : null,
transition: reducedAnimation.value ? 'no' : undefined,
ripple: reducedAnimation.value ? false : undefined,
}
}))

Expand Down
76 changes: 31 additions & 45 deletions src/components/cylc/Drawer.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
<v-navigation-drawer
v-model="drawer"
id="c-sidebar"
ref="drawerRef"
floating
:width="drawerWidth"
class="fill-height"
Expand Down Expand Up @@ -68,11 +67,14 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
</template>

<script>
Copy link
Contributor

@markgrahamdawson markgrahamdawson Jul 10, 2024

Choose a reason for hiding this comment

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

I spent a bit of time yesterday trying to make this into <script setup> but was struggling with these bits

export default {
  components: {
    Workflows,
    'c-header': Header
  },

not sure how to rename a component like this in Vue3

mode: import.meta.env.MODE,
version: pkg.version,

don't know where to put these.

Its not necessary anyway

Copy link
Member

@MetRonnie MetRonnie Jul 10, 2024

Choose a reason for hiding this comment

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

There is no need to specify components in <script setup>, just import them e.g. import CHeader from '@/components/cylc/Header.vue'

You could do const mode = import.meta.env.MODE and just use pkg.version directly in the template

import { nextTick, ref } from 'vue'
import { useDisplay } from 'vuetify'
import Header from '@/components/cylc/Header.vue'
import { mapMutations } from 'vuex'
import Workflows from '@/views/Workflows.vue'
import { mdiHome, mdiGraphql } from '@mdi/js'
import pkg from '@/../package.json'
import { when } from '@/utils'
import { useDrawer } from '@/utils/toolbar'

export const initialWidth = 260
export const minWidth = 150
Expand All @@ -83,71 +85,55 @@ export default {
'c-header': Header
},

data: function () {
return {
drawerWidth: initialWidth
}
},
setup () {
const { mobile } = useDisplay()

mounted () {
this.setEvents()
},
const drawerWidth = ref(initialWidth)

const { drawer } = useDrawer()
// Show drawer initially if viewport is large enough:
drawer.value = !mobile.value

computed: {
drawer: {
get () {
return this.$store.state.app.drawer
},
set (val) {
this.setDrawer(val)
}
function resize (e) {
// If less than min width, will collapse (to 4px because that's the resize-bar width)
drawerWidth.value = e.clientX > minWidth ? e.clientX : 4
}
},

methods: {
...mapMutations('app', ['setDrawer']),
getDrawerElement () {
// Cannot use $refs.drawerRef.$el due to https://github.com/vuetifyjs/vuetify/issues/16766
return document.getElementById('c-sidebar')
},
resize (e) {
// If less than min width, will collapse (to 4px because that's the
// resize-bar width)
this.drawerWidth = e.clientX > minWidth ? e.clientX : 4
},
setEvents () {
const el = this.getDrawerElement()
const drawerBorder = this.$refs.resizeBar
drawerBorder.addEventListener(
/** @type {import('vue').Ref<HTMLElement>} template ref */
const resizeBar = ref(null)

when(resizeBar, () => {
Copy link
Member

Choose a reason for hiding this comment

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

Using when waits until the template ref is available. I think we could use onMounted instead, but maybe not now that some Vuetify components are using <Suspense> - see vuetifyjs/vuetify#19736 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Appears to work ok. The target region for resizing is very narrow and can be obscured by scroll bars, but this isn't new, opened a separate issue.

resizeBar.value.addEventListener(
'mousedown',
(mdEvent) => {
// Prevent Vuetify-provided transitions to ensure responsiveness
el.style.transition = 'none'
Comment on lines -124 to -125
Copy link
Member

Choose a reason for hiding this comment

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

Moved this to CSS

document.body.classList.add('resizing-drawer')
document.addEventListener('mousemove', this.resize, { passive: true })
document.addEventListener('mousemove', resize, { passive: true })
mdEvent.stopPropagation?.()
mdEvent.preventDefault?.()

document.addEventListener(
'mouseup',
(muEvent) => {
if (muEvent.clientX < minWidth) {
this.drawer = false
drawer.value = false
// Reset to width at time of mousedown
// (using a timeout as a hack to prevent drawer briefly
// reappearing (nextTick doesn't work))
setTimeout(() => {
this.drawerWidth = mdEvent.clientX
}, 200)
nextTick(() => {
drawerWidth.value = mdEvent.clientX
})
}
el.style.transition = null
document.body.classList.remove('resizing-drawer')
document.removeEventListener('mousemove', this.resize)
document.removeEventListener('mousemove', resize)
},
{ once: true }
)
}
)
})

return {
drawer,
drawerWidth,
resizeBar,
}
},

Expand Down
4 changes: 2 additions & 2 deletions src/components/cylc/Toolbar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ component. Note: this is not used for the workflow view, see

<script>
import { mapState } from 'vuex'
import { useToolbar, toolbarHeight } from '@/utils/toolbar'
import { useDrawer, toolbarHeight } from '@/utils/toolbar'
import {
mdiViewList
} from '@mdi/js'

export default {
setup () {
const { toggleDrawer } = useToolbar()
const { toggleDrawer } = useDrawer()
return { toggleDrawer, toolbarHeight }
},

Expand Down
5 changes: 3 additions & 2 deletions src/components/cylc/workspace/Toolbar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ import {
} from '@mdi/js'
import { startCase } from 'lodash'
import { until } from '@/utils'
import { useToolbar, toolbarHeight } from '@/utils/toolbar'
import { useDrawer, useNavBtn, toolbarHeight } from '@/utils/toolbar'
import WorkflowState from '@/model/WorkflowState.model'
import graphql from '@/mixins/graphql'
import {
Expand Down Expand Up @@ -264,7 +264,8 @@ export default {
name: 'Toolbar',

setup () {
const { showNavBtn, toggleDrawer } = useToolbar()
const { showNavBtn } = useNavBtn()
const { toggleDrawer } = useDrawer()
return {
eventBus,
showNavBtn,
Expand Down
4 changes: 0 additions & 4 deletions src/store/app.module.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,11 @@
import { markRaw } from 'vue'

const state = () => ({
drawer: null,
Copy link
Member

Choose a reason for hiding this comment

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

1๏ธโƒฃ For some reason, in Vuetify 3.6, trying to base the drawer visibility state off the vuex store broke resizing the drawer: vuetifyjs/vuetify#19925

See next comment for how I worked around this

title: null,
workspaceLayouts: new Map(),
})

const mutations = {
setDrawer (state, drawer) {
state.drawer = drawer
},
setTitle (state, title) {
state.title = title
},
Expand Down
8 changes: 4 additions & 4 deletions src/styles/cylc/_drawer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@

body.resizing-drawer {
cursor: ew-resize !important;
#c-sidebar, .v-main {
// Prevent Vuetify-provided transitions during resize to ensure responsiveness
transition: none !important;
}
}

#c-sidebar {
/* this is not in our styles/material-dashboard, so we need to force-override */
-webkit-box-shadow: none !important;
box-shadow: none !important;
Comment on lines -26 to -28
Copy link
Member

Choose a reason for hiding this comment

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

I think this was outdated, wasn't doing anything

Copy link
Member

Choose a reason for hiding this comment

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

There is a visible drop shadow when opening the draw in the workspace view from its collapsed state (i.e. via the toggle button in the top left), but this is fine.


@include theme-dependent(background-color, settings.$grey, 4);

.resize-bar {
Expand Down
46 changes: 17 additions & 29 deletions src/utils/toolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,46 +15,34 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import { computed, onMounted } from 'vue'
import { computed, ref } from 'vue'
import { useDisplay } from 'vuetify'
import { useStore } from 'vuex'

/** Height in px */
export const toolbarHeight = 48

/**
* Composable that returns a computed property for whether we should show
* the hamburger nav drawer button.
*/
export function useNavBtn () {
const { mobile } = useDisplay()
const store = useStore()
/** Global state of navigation drawer visibility */
const drawer = ref(false)
Comment on lines +24 to +25
Copy link
Member

Choose a reason for hiding this comment

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

2๏ธโƒฃ This replaces the entry in the vuex store


function toggleDrawer () {
drawer.value = !drawer.value
}

/** Composable that provides the global state of the navigation drawer visibility. */
export function useDrawer () {
return {
showNavBtn: computed(() => mobile.value || !store.state.app.drawer),
drawer,
toggleDrawer,
}
}

/**
* Composable that replaces the old toolbar mixin.
*
* Main responsibility is to add responsive toggle
* to a Toolbar component. Shared between (at least) the Cylc
* UI default Toolbar, and the Workflow view Toolbar.
* Composable that returns a computed property for whether we should show
* the hamburger nav drawer button.
*/
export function useToolbar () {
const store = useStore()
const { showNavBtn } = useNavBtn()

onMounted(() => {
store.commit('app/setDrawer', !showNavBtn.value)
})

const toggleDrawer = () => {
store.commit('app/setDrawer', !store.state.app.drawer)
}

export function useNavBtn () {
const { mobile } = useDisplay()
return {
showNavBtn,
toggleDrawer,
showNavBtn: computed(() => mobile.value || !drawer.value),
}
}
18 changes: 12 additions & 6 deletions tests/unit/components/app.vue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,16 +61,22 @@ describe('App', () => {
})

it.each([
{ value: true, expected: { vuetify: false } },
{ value: false, expected: { vuetify: null } },
{
value: true,
expected: {
transition: 'no',
ripple: false
}
},
{
value: false,
expected: {}
},
])('applies reduced animation = $value from localStorage', ({ value, expected }) => {
localStorage.setItem('reducedAnimation', value)
const wrapper = mountFunction()
expect(wrapper.vm.vuetifyDefaults).toMatchObject({
global: {
transition: expected.vuetify,
ripple: expected.vuetify,
}
global: expected
})
})
})
7 changes: 5 additions & 2 deletions tests/unit/components/cylc/workflow/toolbar.vue.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,18 @@ import Toolbar from '@/components/cylc/workspace/Toolbar.vue'
import CommandMenuPlugin from '@/components/cylc/commandMenu/plugin'
import sinon from 'sinon'
import WorkflowService from '@/services/workflow.service'
import { useDrawer } from '@/utils/toolbar'

const $workflowService = sinon.createStubInstance(WorkflowService)

describe('Workspace toolbar component', () => {
let store
const { drawer: drawerState } = useDrawer()

beforeEach(() => {
store = createStore(storeOptions)
store.commit('user/SET_USER', { owner: 'rincewind' })
drawerState.value = false
})

it('hides/shows nav button according to viewport size & whether drawer is collapsed', async () => {
Expand All @@ -47,11 +51,10 @@ describe('Workspace toolbar component', () => {
})
// Btn should show when drawer is collapsed
wrapper.vm.$vuetify.display.mobile = false
store.commit('app/setDrawer', false)
await wrapper.vm.$nextTick()
expect(wrapper.find('#toggle-drawer').exists()).to.equal(true)
// Btn should not show when drawer is visible on large viewport
store.commit('app/setDrawer', true)
drawerState.value = true
await wrapper.vm.$nextTick()
expect(wrapper.find('#toggle-drawer').exists()).to.equal(false)
// Btn should show when drawer is visible on small viewport
Expand Down
13 changes: 0 additions & 13 deletions tests/unit/store/app.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,6 @@ describe('app', () => {
beforeEach(() => {
store = createStore(storeOptions)
})
/**
* Tests for store.app.drawer.
*/
describe('drawer', () => {
it('should start with no drawer', () => {
expect(store.state.app.drawer).to.equal(null)
})
it('should set drawer', () => {
const drawer = true
store.commit('app/setDrawer', drawer)
expect(store.state.app.drawer).to.equal(drawer)
})
})
/**
* Tests for store.app.title.
*/
Expand Down
Loading