-
Notifications
You must be signed in to change notification settings - Fork 156
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
Add viewMode toggle for condensed resource table #7976
Changes from all commits
33bfffd
d7b83cb
8d419a9
f8aa5f5
a42a10f
4f10bd8
bb2e736
9efc6fd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
Enhancement: Add switch to enable condensed resource table | ||
|
||
We've added a switch to have a more condensed resource table. | ||
The change gets saved to the route and persisted across resource navigation. | ||
|
||
https://github.com/owncloud/web/pull/7976 | ||
https://github.com/owncloud/web/issues/6380 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
Enhancement: Add condensed menu icon | ||
|
||
We added a new menu icon for the switch between condensed and regular resource table. | ||
|
||
https://github.com/owncloud/web/pull/7976 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
export abstract class ViewModeConstants { | ||
static readonly default: string = 'resource-table' | ||
static readonly queryName: string = 'view-mode' | ||
static readonly condensedTable: string = 'resource-table-condensed' | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not a huge fan of this use class as enum pattern, I'd rather use typescript types for this nowadays There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've followed what I found in another composable basically, idk if refactoring my/the other solution independently is within the scope of this PR? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with both of you 😆 Would like to get rid of it, but also fine in a separate PR. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
export * from './constants' | ||
export * from './useViewMode' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
import { computed, ComputedRef, unref } from 'vue' | ||
import { queryItemAsString, useRouteQueryPersisted } from 'web-pkg/src/composables' | ||
import { ViewModeConstants } from './constants' | ||
|
||
export function useViewMode<T>(options: ComputedRef<string>): ComputedRef<string> { | ||
if (options) { | ||
return computed(() => unref(options)) | ||
} | ||
|
||
const viewModeQuery = useRouteQueryPersisted({ | ||
name: ViewModeConstants.queryName, | ||
defaultValue: ViewModeConstants.default | ||
}) | ||
return computed(() => queryItemAsString(unref(viewModeQuery))) | ||
} |
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
@@ -1,8 +1,10 @@ | ||||
import { shallowMount, createLocalVue } from '@vue/test-utils' | ||||
import { mount, shallowMount, createLocalVue } from '@vue/test-utils' | ||||
import Vuex from 'vuex' | ||||
import VueRouter from 'vue-router' | ||||
import merge from 'lodash-es/merge' | ||||
|
||||
import DesignSystem from '@ownclouders/design-system' | ||||
|
||||
import Store from 'web-app-files/src/store' | ||||
import stubs from '@/tests/unit/stubs' | ||||
import OcPageSize from '@/tests/unit/stubs/OcPageSize' | ||||
|
@@ -95,4 +97,46 @@ describe('ViewOptions', () => { | |||
|
||||
expect(mockedStore.modules.Files.mutations.SET_HIDDEN_FILES_VISIBILITY).toHaveBeenCalled() | ||||
}) | ||||
|
||||
it('initially shows normal resource-table by default', () => { | ||||
const wrapper = shallowMount(ViewOptions, { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. for future tests (not blocking this PR): we introduced a new way of handling mocks in unit tests. See for example
The functions creating the wrappers now return an object, containing mocks, storeOptions and the wrapper. That gives you the chance to manipulate mocks within the test code. |
||||
store, | ||||
router, | ||||
localVue, | ||||
stubs: stubs, | ||||
directives: { OcTooltip } | ||||
}) | ||||
const viewModeSwitchButtons = wrapper.find('[data-testid="viewmode-switch-buttons"]') | ||||
|
||||
expect(viewModeSwitchButtons).toMatchSnapshot() | ||||
}) | ||||
it('toggles between normal and condensed resource-table upon clicking the respective buttons', async () => { | ||||
localVue.use(DesignSystem) | ||||
|
||||
const wrapper = mount(ViewOptions, { | ||||
store, | ||||
router, | ||||
localVue, | ||||
stubs: { | ||||
...stubs, | ||||
'oc-button': false | ||||
}, | ||||
directives: { OcTooltip } | ||||
}) | ||||
|
||||
const viewModeSwitchButtons = wrapper.find('[data-testid="viewmode-switch-buttons"]') | ||||
console.log(viewModeSwitchButtons.html()) | ||||
|
||||
await wrapper | ||||
.findAll('[data-testid="viewmode-switch-buttons"] > .oc-button') | ||||
.at(0) | ||||
.trigger('click') | ||||
expect(viewModeSwitchButtons).toMatchSnapshot() | ||||
|
||||
await wrapper | ||||
.findAll('[data-testid="viewmode-switch-buttons"] > .oc-button') | ||||
.at(1) | ||||
.trigger('click') | ||||
expect(viewModeSwitchButtons).toMatchSnapshot() | ||||
}) | ||||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`ViewOptions initially shows normal resource-table by default 1`] = ` | ||
<div data-testid="viewmode-switch-buttons" class="oc-button-group oc-visible@s oc-mr-s"> | ||
<oc-button-stub appearance="outline"> | ||
<oc-icon-stub name="menu-line-condensed" fill-type="none" size="small"></oc-icon-stub> | ||
</oc-button-stub> | ||
<oc-button-stub appearance="filled"> | ||
<oc-icon-stub name="menu-line" fill-type="none" size="small"></oc-icon-stub> | ||
</oc-button-stub> | ||
</div> | ||
`; | ||
|
||
exports[`ViewOptions toggles between normal and condensed resource-table upon clicking the respective buttons 1`] = ` | ||
<div data-testid="viewmode-switch-buttons" class="oc-button-group oc-visible@s oc-mr-s"><button type="button" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-filled"> | ||
<oc-icon-stub name="menu-line-condensed" filltype="none" accessiblelabel="" type="span" size="small" variation="passive" color=""></oc-icon-stub> | ||
</button> <button type="button" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-outline"> | ||
<oc-icon-stub name="menu-line" filltype="none" accessiblelabel="" type="span" size="small" variation="passive" color=""></oc-icon-stub> | ||
</button></div> | ||
`; | ||
|
||
exports[`ViewOptions toggles between normal and condensed resource-table upon clicking the respective buttons 2`] = ` | ||
<div data-testid="viewmode-switch-buttons" class="oc-button-group oc-visible@s oc-mr-s"><button type="button" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-outline"> | ||
<oc-icon-stub name="menu-line-condensed" filltype="none" accessiblelabel="" type="span" size="small" variation="passive" color=""></oc-icon-stub> | ||
</button> <button type="button" class="oc-button oc-rounded oc-button-m oc-button-justify-content-center oc-button-gap-m oc-button-passive oc-button-passive-filled"> | ||
<oc-icon-stub name="menu-line" filltype="none" accessiblelabel="" type="span" size="small" variation="passive" color=""></oc-icon-stub> | ||
</button></div> | ||
`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kulmann not sure if the changelog in the design system still gets updated individually or if there's only one in the repo root from now on?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to maintain a separate changelog, because we want to be able to release independently with non aligned version numbers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good, that's what I assumed (and hence the changelog entry^^)