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.

Sign up for GitHub

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

Cleaned up account info page #6547

Merged
merged 1 commit into from
Mar 8, 2022
Merged

Cleaned up account info page #6547

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions changelog/unreleased/enhancement-account-page-loading
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Enhancement: Don't block account page while groups are loading

We don't show a loading state for the full account information page anymore while the group membership information is loading. Instead we only show a loading spinner for the group membership information, while the rest of the user information is available immediately.

https://github.com/owncloud/web/pull/6547
112 changes: 53 additions & 59 deletions packages/web-runtime/src/pages/account.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
<template>
<div class="oc-width-1-1 oc-container oc-p">
<div v-if="loading" class="oc-flex oc-flex-between oc-flex-middle">
<h1 class="oc-page-title">{{ pageTitle }}</h1>
<oc-loader />
</div>
<template v-else>
<div class="oc-flex oc-flex-between oc-flex-middle">
<h1 id="account-page-title" v-translate class="oc-page-title">Account</h1>
<main id="account" class="oc-height-1-1 oc-m">
<div class="oc-flex oc-flex-between oc-flex-bottom oc-width-1-1 oc-border-b oc-py">
<h1 id="account-page-title" class="oc-page-title oc-m-rm">{{ pageTitle }}</h1>
<div>
<oc-button
v-if="editUrl"
variation="primary"
Expand All @@ -28,51 +24,51 @@
<translate>Edit</translate>
</oc-button>
</div>
<hr />
<h2 v-translate class="oc-text-bold oc-text-initial oc-mb">Account Information</h2>

<dl class="account-page-info oc-flex oc-flex-wrap">
<div class="account-page-info-username oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted">Username</dt>
<dd>
{{ user.username || user.id }}
</dd>
</div>
<div v-if="user.username && user.id" class="account-page-info-userid">
<dt v-translate class="oc-text-normal oc-text-muted">User ID</dt>
<dd>
{{ user.id }}
</dd>
</div>
<div class="account-page-info-displayname oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted">Display name</dt>
<dd>
{{ user.displayname }}
</dd>
</div>
<div class="account-page-info-email oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted">Email</dt>
<dd>
<template v-if="user.email">{{ user.email }}</template>
<span v-else v-translate>No email has been set up</span>
</dd>
</div>
<div class="account-page-info-groups oc-mb oc-width-1-2@s">
<dt
v-translate
class="oc-text-normal oc-text-muted"
@click="$_oc_settingsAccount_getGroup"
</div>
<h2 v-translate class="oc-text-bold oc-text-initial oc-mb">Account Information</h2>
<dl class="account-page-info oc-flex oc-flex-wrap">
Copy link
Member Author

@kulmann kulmann Mar 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

mostly indentation changes from here on... only line 59 and following have actually changed.

<div class="account-page-info-username oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted">Username</dt>
<dd>
{{ user.username || user.id }}
</dd>
</div>
<div v-if="user.username && user.id" class="account-page-info-userid">
<dt v-translate class="oc-text-normal oc-text-muted">User ID</dt>
<dd>
{{ user.id }}
</dd>
</div>
<div class="account-page-info-displayname oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted">Display name</dt>
<dd>
{{ user.displayname }}
</dd>
</div>
<div class="account-page-info-email oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted">Email</dt>
<dd>
<template v-if="user.email">{{ user.email }}</template>
<span v-else v-translate>No email has been set up</span>
</dd>
</div>
<div class="account-page-info-groups oc-mb oc-width-1-2@s">
<dt v-translate class="oc-text-normal oc-text-muted" @click="loadGroups">
Group memberships
</dt>
<dd data-testid="group-names">
<oc-spinner
v-if="loadingGroups"
:aria-label="$gettext('Loading group membership information')"
/>
<span v-else-if="groupNames">{{ groupNames }}</span>
<span v-else v-translate data-testid="group-names-empty"
>You are not part of any group</span
>
Group memberships
</dt>
<dd>
<span v-if="groupNames">{{ groupNames }}</span>
<span v-else v-translate>You are not part of any group</span>
</dd>
</div>
</dl>
</template>
</div>
</dd>
</div>
</dl>
</main>
</template>

<script>
Expand All @@ -81,7 +77,7 @@ export default {
name: 'Personal',
data() {
return {
loading: true,
loadingGroups: true,
groups: []
}
},
Expand All @@ -108,14 +104,12 @@ export default {
}
},
mounted() {
this.$_oc_settingsAccount_getGroup()
this.loadGroups()
},
methods: {
$_oc_settingsAccount_getGroup() {
this.$client.users.getUserGroups(this.user.id).then((groups) => {
this.groups = groups
this.loading = false
})
async loadGroups() {
this.groups = await this.$client.users.getUserGroups(this.user.id)
this.loadingGroups = false
}
}
}
Expand Down
21 changes: 8 additions & 13 deletions packages/web-runtime/tests/unit/pages/__snapshots__/account.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`account when the page is not in loading state anymore account information with group information 1`] = `
<div class="account-page-info-groups oc-mb oc-width-1-2@s">
<dt class="oc-text-normal oc-text-muted">
Group memberships
</dt>
<dd><span>one, two, three</span></dd>
</div>
`;

exports[`account when the page is not in loading state anymore account information without group information 1`] = `
exports[`account page account information displays basic user information 1`] = `
<dl class="account-page-info oc-flex oc-flex-wrap">
<div class="account-page-info-username oc-mb oc-width-1-2@s">
<dt class="oc-text-normal oc-text-muted">Username</dt>
Expand All @@ -32,19 +23,23 @@ exports[`account when the page is not in loading state anymore account informati
<dt class="oc-text-normal oc-text-muted">
Group memberships
</dt>
<dd><span>You are not part of any group</span></dd>
<dd data-testid="group-names">
<oc-spinner-stub aria-label="Loading group membership information"></oc-spinner-stub>
</dd>
</div>
</dl>
`;

exports[`account when the page is not in loading state anymore edit buttons edit route button should be displayed if running with ocis and has navItems 1`] = `
exports[`account page account information group membership displays group names 1`] = `<dd data-testid="group-names"><span>one, two, three</span></dd>`;

exports[`account page header section edit buttons edit route button should be displayed if running with ocis and has navItems 1`] = `
<oc-button-stub variation="primary" type="router-link" to="some-route" data-testid="account-page-edit-route-btn">
<oc-icon-stub name="edit"></oc-icon-stub>
<translate-stub tag="span">Edit</translate-stub>
</oc-button-stub>
`;

exports[`account when the page is not in loading state anymore edit buttons edit url button should be displayed if not running with ocis 1`] = `
exports[`account page header section edit buttons edit url button should be displayed if not running with ocis 1`] = `
<oc-button-stub variation="primary" type="a" href="http://server/address/index.php/settings/personal" data-testid="account-page-edit-url-btn">
<oc-icon-stub name="edit"></oc-icon-stub>
<translate-stub tag="span">Edit</translate-stub>
Expand Down
103 changes: 48 additions & 55 deletions packages/web-runtime/tests/unit/pages/account.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,43 +24,21 @@ const $route = {

const selectors = {
pageTitle: '.oc-page-title',
loaderStub: 'oc-loader-stub',
accountPageTitle: '#account-page-title',
loaderStub: 'oc-spinner-stub',
editUrlButton: '[data-testid="account-page-edit-url-btn"]',
editRouteButton: '[data-testid="account-page-edit-route-btn"]',
accountPageInfo: '.account-page-info',
groupInformation: '.account-page-info-groups'
groupNames: '[data-testid="group-names"]',
groupNamesEmpty: '[data-testid="group-names-empty"]'
}

describe('account', () => {
describe('when the page is still loading', () => {
let wrapper
beforeEach(() => {
wrapper = getWrapper()
wrapper.setData({ loading: true })
})
it('should render the loading state', () => {
describe('account page', () => {
describe('header section', () => {
it('renders page title', () => {
const wrapper = getWrapper()
const pageTitle = wrapper.find(selectors.pageTitle)
const ocLoader = wrapper.find(selectors.loaderStub)
expect(pageTitle.exists()).toBeTruthy()
expect(pageTitle.text()).toBe($route.meta.title)
expect(ocLoader.exists()).toBeTruthy()
})
it('should not render the account page content', () => {
const accountPageTitle = wrapper.find(selectors.accountPageTitle)
const accountPageInfo = wrapper.find(selectors.accountPageInfo)
expect(accountPageTitle.exists()).toBeFalsy()
expect(accountPageInfo.exists()).toBeFalsy()
})
})
describe('when the page is not in loading state anymore', () => {
it('should not render the loading state', async () => {
const store = getStore()
const wrapper = getWrapper(store)
await wrapper.setData({ loading: false })
const pageTitle = wrapper.find(selectors.pageTitle)
const ocLoader = wrapper.find(selectors.loaderStub)
expect(pageTitle.text()).toBe('Account')
expect(ocLoader.exists()).toBeFalsy()
})

describe('edit buttons', () => {
Expand All @@ -71,7 +49,7 @@ describe('account', () => {
isOcis: false
})
const wrapper = getWrapper(store)
await wrapper.setData({ loading: false })
await wrapper.setData({ loadingGroups: false })
const editUrlButton = wrapper.find(selectors.editUrlButton)
const editRouteButton = wrapper.find(selectors.editRouteButton)
expect(editUrlButton).toMatchSnapshot()
Expand All @@ -83,9 +61,9 @@ describe('account', () => {
isOcis: true
})
const wrapper = getWrapper(store)
await wrapper.setData({ loading: false })
const editButton = wrapper.find(selectors.editUrlButton)
expect(editButton.exists()).toBeFalsy()
await wrapper.setData({ loadingGroups: false })
const editUrlButton = wrapper.find(selectors.editUrlButton)
expect(editUrlButton.exists()).toBeFalsy()
})
})
describe('edit route button', () => {
Expand All @@ -95,42 +73,57 @@ describe('account', () => {
getNavItemsByExtension: jest.fn(() => [{ route: 'some-route' }])
})
const wrapper = getWrapper(store)
await wrapper.setData({ loading: false })
await wrapper.setData({ loadingGroups: false })
const editRouteButton = wrapper.find(selectors.editRouteButton)
expect(editRouteButton).toMatchSnapshot()
})
})
})
})

describe('account information', () => {
it('without group information', async () => {
const store = getStore({
user: {
username: 'some-username',
displayname: 'some-displayname',
email: 'some-email'
}
})
describe('account information', () => {
it('displays basic user information', async () => {
const store = getStore({
user: {
username: 'some-username',
displayname: 'some-displayname',
email: 'some-email'
}
})
const wrapper = getWrapper(store)

const accountPageInfo = wrapper.find(selectors.accountPageInfo)
expect(accountPageInfo).toMatchSnapshot()
})

describe('group membership', () => {
it('shows loading indicator when in loading state', () => {
const wrapper = getWrapper()
const groupInfoLoading = wrapper.find(selectors.loaderStub)
expect(groupInfoLoading.exists()).toBeTruthy()
})
it('displays message if not member of any groups', async () => {
const store = getStore()
const wrapper = getWrapper(store)
await wrapper.setData({ loading: false })
await wrapper.setData({ loadingGroups: false })

const accountPageInfo = wrapper.find(selectors.accountPageInfo)
expect(accountPageInfo).toMatchSnapshot()
const groupNamesEmpty = wrapper.find(selectors.groupNamesEmpty)
expect(groupNamesEmpty.exists()).toBeTruthy()
})
it('with group information', async () => {
it('displays group names', async () => {
const store = getStore()
const wrapper = getWrapper(store)
await wrapper.setData({ loading: false })
await wrapper.setData({ loadingGroups: false })
await wrapper.setData({ groups: ['one', 'two', 'three'] })

const groupInformation = wrapper.find(selectors.groupInformation)
expect(groupInformation).toMatchSnapshot()
const groupNames = wrapper.find(selectors.groupNames)
expect(groupNames).toMatchSnapshot()
})
})
})
})

function getWrapper(store = null) {
function getWrapper(store = getStore()) {
const component = {
...account,
mounted: jest.fn()
Expand All @@ -141,12 +134,12 @@ function getWrapper(store = null) {
$route
},
stubs: {
'oc-loader': true,
'oc-spinner': true,
'oc-button': true,
'oc-icon': true
}
},
store
}
if (store) opts.store = store
return shallowMount(component, opts)
}

Expand Down