Skip to content

Commit

Permalink
check if everything is ready before rendering the ui
Browse files Browse the repository at this point in the history
show loading screen while waiting for all required data from backend
check for route path instead of name in ADD_NAV_ITEM
fix session persist and add only what is required
  • Loading branch information
fschade committed Dec 10, 2020
1 parent 68f9330 commit 2cee264
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 48 deletions.
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Bugfix: copy objects
Bugfix: fix navigation rendering

in some cases objects needs to be copied before mutating them:
- ADD_NAV_ITEM mutation now gets copied instead of referenced to trigger a state change.
- applicationsList navItem item needs a copy instead of mutating the base item
- check for route.path instead of route name in ADD_NAV_ITEM which can change over time

https://github.com/owncloud/ocis/issues/1031
https://github.com/owncloud/phoenix/pull/4430
Expand Down
9 changes: 9 additions & 0 deletions changelog/unreleased/phoenix-ui-is-ready
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Enhancement: wait for all required data

Before this we rendered the ui no matter if every required data already is loaded or not.
For example the current users language from the ocis settings service.
One potential problem was the flickering in the ui or that the default language was shown before it switches to the settings language of current user.
Instead we now show a loading screen and wait for everything that is required before rendering anything else.

https://github.com/owncloud/ocis/issues/884
https://github.com/owncloud/ocis/issues/1043
40 changes: 31 additions & 9 deletions src/Phoenix.vue
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,17 @@
<div class="uk-height-1-1">
<skip-to target="main">Skip to main</skip-to>
<div id="Phoenix" class="uk-height-1-1">
<template v-if="!showHeader">
<div
v-if="user.isAuthenticated && !user.userReady"
class="loading-overlay"
:style="{
backgroundImage: 'url(' + configuration.theme.loginPage.backgroundImg + ')'
}"
uk-height-viewport
>
<oc-spinner size="xxxlarge" :aria-label="$gettext('Loading')" class="uk-position-center" />
</div>
<template v-else-if="!showHeader">
<router-view name="fullscreen" />
</template>
<div v-else key="core-content" class="uk-height-1-1 uk-flex uk-flex-row uk-flex-row">
Expand Down Expand Up @@ -124,7 +134,6 @@ export default {
return list
},
showHeader() {
return this.$route.meta.hideHeadbar !== true
},
Expand Down Expand Up @@ -162,13 +171,11 @@ export default {
return item.enabled(this.capabilities)
})
return items.map(item => {
item = { ...item }
item.name = this.$gettext(item.name)
item.active = this.$route.name === item.route.name
return item
})
return items.map(item => ({
...item,
name: this.$gettext(item.name),
active: this.$route.name === item.route.name
}))
},
sidebarClasses() {
Expand Down Expand Up @@ -320,4 +327,19 @@ body {
height: 100vh;
overflow: hidden;
}
.loading-overlay {
background-size: cover;
background-repeat: no-repeat;
background-position: 50%;
}
.loading-overlay .oc-spinner {
color: #0a264e;
}
.loading-overlay .oc-spinner:after {
border: 20px solid;
border-bottom: 20px solid transparent;
}
</style>
11 changes: 7 additions & 4 deletions src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ const vuexPersistInSession = new VuexPersistence({
key: 'phoenixStateInSessionStorage',
// Browser tab independent storage which gets deleted after the tab is closed
storage: window.sessionStorage,
filter: mutation =>
['SAVE_URL_BEFORE_LOGIN', 'SET_USER', 'SET_TOKEN', 'SET_CAPABILITIES'].indexOf(mutation.type) >
-1,
modules: ['router', 'user']
reducer: state => {
const { userReady, ...user } = state.user
return {
user,
router: state.router
}
}
})

const strict = process.env.NODE_ENV === 'development'
Expand Down
4 changes: 3 additions & 1 deletion src/store/navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ const mutations = {
ADD_NAV_ITEM(state, { extension, navItem }) {
const dynamicNavItems = { ...state.dynamicNavItems }
dynamicNavItems[extension] = dynamicNavItems[extension] || []
const index = dynamicNavItems[extension].findIndex(item => item.name === navItem.name)
const index = dynamicNavItems[extension].findIndex(
item => item.route.path === navItem.route.path
)
if (index >= 0) {
dynamicNavItems[extension][index] = navItem
} else {
Expand Down
72 changes: 40 additions & 32 deletions src/store/user.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ const state = {
isAuthenticated: false,
capabilities: [],
version: {},
groups: []
groups: [],
userReady: false
}

const actions = {
Expand All @@ -23,6 +24,9 @@ const actions = {
context.commit('SET_USER', state)
// reset capabilities to default state
context.commit('SET_CAPABILITIES', { capabilities: null, version: null })
// set userReady to false
context.commit('SET_USER_READY', false)

// clear oidc client state
vueAuthInstance.clearLoginState()
},
Expand Down Expand Up @@ -56,8 +60,8 @@ const actions = {
logoutFinalizier(true)
}
},
initAuth(context, payload = { autoRedirect: false }) {
function init(client, token, doLogin = true) {
async initAuth(context, payload = { autoRedirect: false }) {
const init = async (client, token, doLogin = true) => {
const instance = context.rootState.config.server || window.location.origin
const options = {
baseUrl: instance,
Expand All @@ -78,39 +82,40 @@ const actions = {

client.init(options)
if (doLogin) {
return client
.login()
.then(res => {
return client.getCapabilities().then(cap => {
client.users.getUserGroups(res.id).then(groups => {
context.commit('SET_CAPABILITIES', cap)
context.commit('SET_USER', {
id: res.id,
username: res.username,
displayname: res.displayname || res['display-name'],
email: !Object.keys(res.email).length ? '' : res.email,
token,
isAuthenticated: true,
groups: groups
})
context.dispatch('loadSettingsValues')
let login
try {
login = await client.login()
} catch (e) {
console.warn('Seems that your token is invalid. Error:', e)
context.dispatch('cleanUpLoginState')
router.push({ name: 'accessDenied' })
return
}

const capabilities = await client.getCapabilities()
context.commit('SET_CAPABILITIES', capabilities)

const userGroups = await client.users.getUserGroups(login.id)
context.commit('SET_USER', {
id: login.id,
username: login.username,
displayname: login.displayname || login['display-name'],
email: !Object.keys(login.email).length ? '' : login.email,
token,
isAuthenticated: true,
groups: userGroups
})

if (payload.autoRedirect) {
router.push({ path: '/' })
}
})
})
})
.catch(e => {
console.warn('Seems that your token is invalid. Error:', e)
context.dispatch('cleanUpLoginState')
router.push({ name: 'accessDenied' })
})
await context.dispatch('loadSettingsValues')
context.commit('SET_USER_READY', true)

if (payload.autoRedirect) {
router.push({ path: '/' })
}
} else {
context.commit('UPDATE_TOKEN', token)
}
}

// if called from login, use available vue-authenticate instance; else re-init
if (!vueAuthInstance) {
vueAuthInstance = initVueAuthenticate(context.rootState.config)
Expand Down Expand Up @@ -142,7 +147,7 @@ const actions = {
}
const token = vueAuthInstance.getToken()
if (token) {
init(this._vm.$client, token)
await init(this._vm.$client, token)
}
},
login(context, payload = { provider: 'oauth2' }) {
Expand Down Expand Up @@ -192,6 +197,9 @@ const mutations = {
},
UPDATE_TOKEN(state, token) {
state.token = token
},
SET_USER_READY(state, ready) {
state.userReady = ready
}
}

Expand Down

0 comments on commit 2cee264

Please sign in to comment.