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 ocis-1043 #4444

Merged
merged 5 commits into from
Dec 11, 2020
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
6 changes: 0 additions & 6 deletions changelog/unreleased/fix-nav-item-state

This file was deleted.

9 changes: 9 additions & 0 deletions changelog/unreleased/fix-navigation-rendering
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
Bugfix: fix navigation rendering

- 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
https://github.com/owncloud/ocis/issues/1043
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
39 changes: 31 additions & 8 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="xlarge" :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,12 +171,11 @@ export default {
return item.enabled(this.capabilities)
})

return items.map(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 @@ -319,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: 10px solid;
border-bottom: 10px 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
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for spotting this ❤️

)
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)
kulmann marked this conversation as resolved.
Show resolved Hide resolved
}
},
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