fix(ready): remove class from fragment #1276

Merged
dpschen merged 1 commits from dpschen/frontend:featur/fix-remove-class-from-fragment into main 2022-01-04 20:34:34 +00:00
6 changed files with 45 additions and 30 deletions

View File

@ -1,32 +1,27 @@
<template>
<ready>
<div :class="{'is-touch': isTouch}">
konrad marked this conversation as resolved Outdated

What about this? Is the banner still shown when the user goes offline?

What about this? Is the banner still shown when the user goes offline?

Because of these code lines in the <ready> component the is-hidden class is useless since the slot content would never mount if the user is offline:

	<div class="app offline" v-if="!online">
		<div class="offline-message">
			<h1>{{ $t('offline.title') }}</h1>
			<p>{{ $t('offline.text') }}</p>
		</div>
	</div>
	<template v-else-if="ready">
		<slot/>
	</template>
Because of these code lines in the `<ready>` component the `is-hidden` class is useless since the slot content would never mount if the user is offline: ```vue <div class="app offline" v-if="!online"> <div class="offline-message"> <h1>{{ $t('offline.title') }}</h1> <p>{{ $t('offline.text') }}</p> </div> </div> <template v-else-if="ready"> <slot/> </template> ```

Ah, makes sense! I guess this is an old one from before the ready component.

Ah, makes sense! I guess this is an old one from before the ready component.
<div :class="{'is-hidden': !online}">
<template v-if="authUser">
<top-navigation/>
<content-auth/>
</template>
<content-link-share v-else-if="authLinkShare"/>
<no-auth-wrapper v-else>
<router-view/>
</no-auth-wrapper>
<Notification/>
</div>
<template v-if="authUser">
dpschen marked this conversation as resolved Outdated

Do we really need that extra div?

Do we really need that extra div?

It's necessary in the sense that we currently don't have a wrapper div around the app that we could use to add a class (in this case is-touch) because ready is as said a fragment.

But instead of adding a additional div we can also add this class to the body tag. Will do this instead.

It's necessary in the sense that we currently don't have a wrapper div around the app that we could use to add a class (in this case `is-touch`) because `ready` is as said a fragment. **But** instead of adding a additional div we can also add this class to the body tag. Will do this instead.
<top-navigation/>
<content-auth/>
</template>
<content-link-share v-else-if="authLinkShare"/>
<no-auth-wrapper v-else>
<router-view/>
</no-auth-wrapper>
<Notification/>
<transition name="fade">
<keyboard-shortcuts v-if="keyboardShortcutsActive"/>
</transition>
</div>
</ready>
</template>
<script lang="ts" setup>
import {computed, watch, watchEffect, Ref} from 'vue'
import {computed, watch, Ref} from 'vue'
import {useRouter} from 'vue-router'
import {useRouteQuery} from '@vueuse/router'
import {useStore} from 'vuex'
import {useI18n} from 'vue-i18n'
import {useOnline} from '@vueuse/core'
import isTouchDevice from 'is-touch-device'
import {success} from '@/message'
@ -40,17 +35,14 @@ import Ready from '@/components/misc/ready.vue'
import {setLanguage} from './i18n'
import AccountDeleteService from '@/services/accountDelete'
import {ONLINE} from '@/store/mutation-types'
import {useColorScheme} from '@/composables/useColorScheme'
import {useBodyClass} from '@/composables/useBodyClass'
const store = useStore()
const online = useOnline()
watchEffect(() => store.commit(ONLINE, online.value))
const router = useRouter()
const isTouch = computed(isTouchDevice)
useBodyClass('is-touch', isTouchDevice)
const keyboardShortcutsActive = computed(() => store.state.keyboardShortcutsActive)
const authUser = computed(() => store.getters['auth/authUser'])

View File

@ -50,11 +50,12 @@ import Message from '@/components/misc/message.vue'
import NoAuthWrapper from '@/components/misc/no-auth-wrapper.vue'
import {ERROR_NO_API_URL} from '@/helpers/checkAndSetApiUrl'
import {useOnline} from '@/composables/useOnline'
const store = useStore()
const ready = computed(() => store.state.vikunjaReady)
const online = computed(() => store.state.online)
const online = useOnline()
const error = ref('')
const showLoading = computed(() => !ready.value && error.value === '')

View File

@ -0,0 +1,16 @@
import {ref, watchEffect} from 'vue'
import {tryOnBeforeUnmount} from '@vueuse/core'
export function useBodyClass(className: string, defaultValue = false) {
dpschen marked this conversation as resolved Outdated

Nice one!

Nice one!
const isActive = ref(defaultValue)
watchEffect(() => {
isActive.value
? document.body.classList.add(className)
: document.body.classList.remove(className)
})
tryOnBeforeUnmount(() => isActive.value && document.body.classList.remove(className))
return isActive
}

View File

@ -0,0 +1,14 @@
import {ref} from 'vue'
import {useOnline as useNetworkOnline, ConfigurableWindow} from '@vueuse/core'
export function useOnline(options?: ConfigurableWindow) {
const fakeOnlineState = !!import.meta.env.VITE_IS_ONLINE
if (fakeOnlineState) {
console.log('Setting fake online state', fakeOnlineState)
}
return fakeOnlineState
? ref(true)
: useNetworkOnline(options)
}

View File

@ -7,7 +7,7 @@ import {
LOADING,
LOADING_MODULE,
MENU_ACTIVE,
ONLINE, QUICK_ACTIONS_ACTIVE,
QUICK_ACTIONS_ACTIVE,
} from './mutation-types'
import config from './modules/config'
import auth from './modules/auth'
@ -36,7 +36,6 @@ export const store = createStore({
state: {
loading: false,
loadingModule: null,
online: true,
// This is used to highlight the current list in menu for all list related views
currentList: {id: 0},
background: '',
@ -53,12 +52,6 @@ export const store = createStore({
[LOADING_MODULE](state, module) {
state.loadingModule = module
},
[ONLINE](state, online) {
if (import.meta.env.VITE_IS_ONLINE) {
console.log('Setting fake online state', import.meta.env.VITE_IS_ONLINE)
}
state.online = !!import.meta.env.VITE_IS_ONLINE || online
},
[CURRENT_LIST](state, currentList) {
// Server updates don't return the right. Therefore the right is reset after updating the list which is
// confusing because all the buttons will disappear in that case. To prevent this, we're keeping the right

View File

@ -1,6 +1,5 @@
export const LOADING = 'loading'
export const LOADING_MODULE = 'loadingModule'
export const ONLINE = 'online'
export const CURRENT_LIST = 'currentList'
export const HAS_TASKS = 'hasTasks'
export const MENU_ACTIVE = 'menuActive'