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

Add new ThemeSwitcher logic #9698

Merged
merged 23 commits into from
Dec 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
b14361e
Move themes to separate pinia store
pascalwengerter Nov 7, 2023
b916700
Move theme pinia store to web-pkg
pascalwengerter Nov 7, 2023
efb42cb
Next iteration
pascalwengerter Nov 7, 2023
9493526
Add themeSwitcher for more >2 themes as OcDrop, further streamline th…
pascalwengerter Nov 7, 2023
81f5735
Next iterations
pascalwengerter Nov 7, 2023
d6057f6
Rebase and update theme logic handling
pascalwengerter Dec 5, 2023
e5f9e0e
Fix some pinia reactivity issues
pascalwengerter Dec 5, 2023
8200545
Address code review
pascalwengerter Dec 5, 2023
44854b6
Restructure theme.json and loading to reintroduce top-level common se…
pascalwengerter Dec 5, 2023
8831f15
Use zod to parse theme
pascalwengerter Dec 6, 2023
38b1017
Fix logical error and improve typing in theme validation
pascalwengerter Dec 6, 2023
84d34e0
Log ZOD error when validating theme
pascalwengerter Dec 6, 2023
ccf1628
Use zod parse instead of safeParse and use try/catch
pascalwengerter Dec 6, 2023
f4bf391
Fix most unit tests
pascalwengerter Dec 8, 2023
0233377
Small refactorings after code review
pascalwengerter Dec 8, 2023
1de5a55
test: fix unit tests
JammingBen Dec 11, 2023
2544143
test: temporarily disable logo e2e test
JammingBen Dec 11, 2023
9669005
fix: remove app banner config
JammingBen Dec 11, 2023
58dd4ec
fix: favicon meta tag
JammingBen Dec 11, 2023
ccf3493
fix: app banner zod
JammingBen Dec 11, 2023
13e5308
test: allow setting appBanner theme config in tests
JammingBen Dec 11, 2023
2513032
fix: theme store definition
JammingBen Dec 11, 2023
60e0822
feat: use palette icon
JammingBen Dec 11, 2023
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
18 changes: 18 additions & 0 deletions changelog/unreleased/change-theme-handling
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
Change: Theme handling

The handling of themes has been throughoutly reworked.
Themes now feature a human-readable title, can specify whether they are suitable as light or dark mode themes.
If only one theme is provided, this theme will be set.
If two themes are provided and one is a dark and one is a light mode one, there is a light/dark mode switch button in the AppBar.
If more than two themes are provided or the two provided themes are not a dark and a light mode one respectively,
the UI features a theme-switch button featuring a dropdown of the human-readable names of the available themes

https://github.com/owncloud/web/issues/2404
https://github.com/owncloud/web/issues/8424
https://github.com/owncloud/web/issues/9403
https://github.com/owncloud/web/issues/9885
https://github.com/owncloud/web/issues/9939
https://github.com/owncloud/web/pull/8855
https://github.com/owncloud/web/pull/9396
https://github.com/owncloud/web/pull/9401
https://github.com/owncloud/web/pull/9698
2 changes: 0 additions & 2 deletions docs/getting-started.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,6 @@ Depending on the backend you are using, there are sample config files provided i
- `options.tokenStorageLocal` Specifies whether the access token will be stored in the local storage when set to `true` or in the session storage when set to `false`. If stored in the local storage, login state will be persisted across multiple browser tabs, means no additional logins are required. Defaults to `true`.
- `options.loginUrl` Specifies the target URL to the login page. This is helpful when an external IdP is used. This option is disabled by default. Example URL like: 'https://www.myidp.com/login'.
- `options.logoutUrl` Adds a link to the user's profile page to point him to an external page, where he can manage his session and devices. This is helpful when an external IdP is used. This option is disabled by default.
- `options.imprintUrl` Specifies the target URL for the imprint link valid for the ocis instance in the account menu.
- `options.privacyUrl` Specifies the target URL for the privacy link valid for the ocis instance in the account menu.
Comment on lines -82 to -83
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess the config for this has been removed intentionally? Because then we need to remove it in oCIS as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, we should. The way to go is the theme, so that all clients can use the URLs. Config is only for web.

- `options.ocm.openRemotely` Specifies whether opening files in remote shares in their original ocm instance should be enabled. Defaults to `false`.
- `options.userListRequiresFilter` Defines whether one or more filters must be set in order to list users in the Web admin settings. Set this option to 'true' if running in an environment with a lot of users and listing all users could slow down performance. Defaults to `false`.

Expand Down
1 change: 1 addition & 0 deletions packages/web-app-admin-settings/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"design-system": "workspace:@ownclouders/design-system@*",
"fuse.js": "6.6.2",
"lodash-es": "4.17.21",
"pinia": "2.1.7",
"uuid": "9.0.1",
"vue-concurrency": "4.0.1",
"vuex": "4.1.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
</div>
<div>
<div class="logo-wrapper">
<img alt="" :src="logo" class="oc-p-xs" />
<img alt="" :src="currentTheme.logo.topbar" class="oc-p-xs" />
</div>
<input
id="logo-upload-input"
Expand All @@ -44,13 +44,14 @@

<script lang="ts">
import { defineComponent, computed, unref, VNodeRef, ref } from 'vue'
import { ContextActionMenu } from '@ownclouders/web-pkg'
import { ContextActionMenu, useThemeStore } from '@ownclouders/web-pkg'
import {
useGeneralActionsResetLogo,
useGeneralActionsUploadLogo
} from '../../composables/actions/general'
import { supportedLogoMimeTypes } from '../../defaults'
import { useStore } from '@ownclouders/web-pkg'
import { storeToRefs } from 'pinia'

export default defineComponent({
name: 'AppearanceSection',
Expand All @@ -59,6 +60,8 @@ export default defineComponent({
},
setup() {
const store = useStore()
const themeStore = useThemeStore()
const { currentTheme } = storeToRefs(themeStore)

const logoInput: VNodeRef = ref(null)

Expand All @@ -75,7 +78,6 @@ export default defineComponent({
resources: unref(menuItems)
}))

const logo = computed(() => store.getters.configuration.currentTheme.logo.topbar)
const menuSections = computed(() => [
{
name: 'primaryActions',
Expand All @@ -87,7 +89,7 @@ export default defineComponent({

return {
actionOptions,
logo,
currentTheme,
menuItems,
menuSections,
supportedLogoMimeTypesAcceptValue,
Expand Down
1 change: 1 addition & 0 deletions packages/web-app-files/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
"luxon": "3.0.1",
"marked": "^4.0.12",
"p-queue": "^6.6.2",
"pinia": "2.1.7",
"qs": "6.11.2",
"sanitize-html": "^2.7.0",
"uuid": "9.0.1",
Expand Down
12 changes: 9 additions & 3 deletions packages/web-app-files/src/views/FilesDrop.vue
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@
</div>

<div class="oc-text-center oc-mt-xxl">
<p v-text="configuration.currentTheme.general.slogan" />
<p v-text="themeSlogan" />
</div>
</div>
</div>
</template>

<script lang="ts">
import { storeToRefs } from 'pinia'
import { mapGetters } from 'vuex'
import { createLocationPublic, createLocationSpaces } from '@ownclouders/web-pkg'
import { createLocationPublic, createLocationSpaces, useThemeStore } from '@ownclouders/web-pkg'
import ResourceUpload from '../components/AppBar/Upload/ResourceUpload.vue'
import {
computed,
Expand Down Expand Up @@ -78,6 +79,7 @@ export default defineComponent({
setup() {
const uppyService = useService<UppyService>('$uppyService')
const store = useStore()
const themeStore = useThemeStore()
const router = useRouter()
const route = useRoute()
const language = useGettext()
Expand All @@ -89,6 +91,9 @@ export default defineComponent({
const { getInternalSpace } = useGetMatchingSpace()
useUpload({ uppyService })

const { currentTheme } = storeToRefs(themeStore)
const themeSlogan = computed(() => currentTheme.value.common.slogan)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this need to be computed, as storeToRefs builds already a reactive object, or does this not apply to nested objects?

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I understand it: It's not necessary in this case. currentTheme is already a Ref because of storeToRefs. Meaning the consuming code could go directly via unref(currentTheme).common.slogan.

It doesn't hurt to have a computed here though, I guess it's used as a "shortcut".


const fileIdQueryItem = useRouteQuery('fileId')
const fileId = computed(() => {
return queryItemAsString(unref(fileIdQueryItem))
Expand Down Expand Up @@ -207,7 +212,8 @@ export default defineComponent({
dragareaEnabled,
loading,
errorMessage,
share
share,
themeSlogan
}
},
computed: {
Expand Down
3 changes: 2 additions & 1 deletion packages/web-app-files/tests/unit/views/FilesDrop.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from 'web-test-helpers'
import { mock, mockDeep } from 'jest-mock-extended'
import { ClientService } from '@ownclouders/web-pkg'
import { createMockThemeStore } from 'web-test-helpers/src/mocks/pinia'

describe('FilesDrop view', () => {
describe('different files view states', () => {
Expand Down Expand Up @@ -44,7 +45,7 @@ function getMountedWrapper() {
storeOptions,
wrapper: mount(FilesDrop, {
global: {
plugins: [...defaultPlugins(), store],
plugins: [...defaultPlugins(), store, createMockThemeStore()],
mocks: defaultMocks,
provide: defaultMocks,
stubs: defaultStubs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
} from 'web-test-helpers'
import { ConfigurationManager, useBreadcrumbsFromPath } from '@ownclouders/web-pkg'
import { useBreadcrumbsFromPathMock } from '../../../mocks/useBreadcrumbsFromPathMock'
import { createMockThemeStore } from 'web-test-helpers/src/mocks/pinia'

const mockCreateFolder = jest.fn()
const mockUseEmbedMode = jest.fn().mockReturnValue({ isEnabled: computed(() => false) })
Expand Down Expand Up @@ -306,7 +307,6 @@ function getMountedWrapper({
...defaultStoreMockOptions.getters,
configuration: function () {
return {
currentTheme: { general: { slogan: 'Public link slogan' } },
options: {
runningOnEos
}
Expand All @@ -332,7 +332,7 @@ function getMountedWrapper({
wrapper: mount(GenericSpace, {
props: propsData,
global: {
plugins: [...defaultPlugins(), store],
plugins: [...defaultPlugins(), store, createMockThemeStore()],
mocks: defaultMocks,
provide: defaultMocks,
stubs: { ...defaultStubs, 'resource-details': true, portal: true, ...stubs }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
defaultStubs,
RouteLocation
} from 'web-test-helpers'
import { createMockThemeStore } from 'web-test-helpers/src/mocks/pinia'

jest.mock('web-app-files/src/composables')

Expand Down Expand Up @@ -82,7 +83,7 @@ function getMountedWrapper({ mocks = {}, props = {}, files = [], loading = false
wrapper: mount(GenericTrash, {
props: propsData,
global: {
plugins: [...defaultPlugins(), store],
plugins: [...defaultPlugins(), store, createMockThemeStore()],
mocks: defaultMocks,
stubs: { ...defaultStubs, portal: true }
}
Expand Down
3 changes: 2 additions & 1 deletion packages/web-app-importer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"peerDependencies": {
"@ownclouders/design-system": "workspace:*",
"@ownclouders/web-client": "workspace:*",
"@ownclouders/web-pkg": "workspace:*"
"@ownclouders/web-pkg": "workspace:*",
"pinia": "^2.1.7"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps we should re-export the required part (storeToRefs) from web-pkg?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO it's fine to have pinia as peerDependency in most of our packages since that will probably be the case anyway after fully switching to it.

}
}
8 changes: 5 additions & 3 deletions packages/web-app-importer/src/extensions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { useStore, usePublicLinkContext } from '@ownclouders/web-pkg'
import { storeToRefs } from 'pinia'
import { useStore, usePublicLinkContext, useThemeStore } from '@ownclouders/web-pkg'
import { useGettext } from 'vue3-gettext'
import { useService } from '@ownclouders/web-pkg'
import { computed, unref } from 'vue'
Expand All @@ -17,6 +18,8 @@ export const extensions = ({ applicationConfig }: ApplicationSetupOptions) => {
const { $gettext } = useGettext()
const uppyService = useService<UppyService>('$uppyService')
const publicLinkContext = usePublicLinkContext({ store })
const themeStore = useThemeStore()
const { currentTheme } = storeToRefs(themeStore)

const { companionUrl, webdavCloudType } = applicationConfig
let { supportedClouds } = applicationConfig
Expand Down Expand Up @@ -51,8 +54,7 @@ export const extensions = ({ applicationConfig }: ApplicationSetupOptions) => {
})

const handler = async () => {
const currentThemeName = window.localStorage.getItem('oc_currentThemeName')
const renderDarkTheme = currentThemeName === 'default-dark'
const renderDarkTheme = currentTheme.value.isDark

const modal = {
variation: 'passive',
Expand Down
5 changes: 3 additions & 2 deletions packages/web-pkg/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@
"@uppy/xhr-upload": "^3.0.1",
"@vueuse/core": "^10.0.0",
"axios": "1.6.2",
"deepmerge": "^4.2.2",
"design-system": "workspace:@ownclouders/design-system@*",
"dompurify": "^3.0.6",
"filesize": "^9.0.11",
"fuse.js": "6.6.2",
"js-generate-password": "^0.1.9",
Expand All @@ -45,7 +47,6 @@
"vue-concurrency": "4.0.1",
"vue-router": "4.2.5",
"vue3-gettext": "2.4.0",
"vuex": "4.1.0",
"dompurify": "^3.0.6"
"vuex": "4.1.0"
}
}
36 changes: 22 additions & 14 deletions packages/web-pkg/src/components/AppBanner.vue
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,17 @@
</oc-button>
<div
class="app-banner-icon"
:style="{ 'background-image': `url('${appBannerSettings.icon}')` }"
:style="{ 'background-image': `url('${currentTheme.appBanner.icon}')` }"
></div>
<div class="info-container">
<div>
<div class="app-title">{{ appBannerSettings.title }}</div>
<div class="app-publisher">{{ appBannerSettings.publisher }}</div>
<div v-if="appBannerSettings.additionalInformation !== ''" class="app-additional-info">
{{ $gettext(appBannerSettings.additionalInformation) }}
<div class="app-title">{{ currentTheme.appBanner.title }}</div>
<div class="app-publisher">{{ currentTheme.appBanner.publisher }}</div>
<div
v-if="currentTheme.appBanner.additionalInformation !== ''"
class="app-additional-info"
>
{{ $gettext(currentTheme.appBanner.additionalInformation) }}
</div>
</div>
</div>
Expand All @@ -28,18 +31,19 @@
target="_blank"
class="app-banner-cta"
rel="noopener"
aria-label="{{ $gettext(appBannerSettings.ctaText) }}"
>{{ $gettext(appBannerSettings.ctaText) }}</a
aria-label="{{ $gettext(currentTheme.appBanner.ctaText) }}"
>{{ $gettext(currentTheme.appBanner.ctaText) }}</a
>
</div>
</portal>
</template>

<script lang="ts">
import { computed, defineComponent, ref, unref } from 'vue'
import { useRouter, useStore } from '../composables'
import { useRouter, useThemeStore } from '../composables'
import { buildUrl } from '../helpers/router'
import { useSessionStorage } from '@vueuse/core'
import { storeToRefs } from 'pinia'

export default defineComponent({
components: {},
Expand All @@ -52,16 +56,20 @@ export default defineComponent({
setup(props) {
const appBannerWasClosed = useSessionStorage('app_banner_closed', null)
const isVisible = ref<boolean>(unref(appBannerWasClosed) === null)
const store = useStore()

const router = useRouter()
const appBannerSettings = unref(store.getters.configuration.currentTheme.appBanner)
const themeStore = useThemeStore()
const { currentTheme } = storeToRefs(themeStore)

const appBannerSettings = currentTheme.value.appBanner
const isAppBannerAvailable = computed(
() => appBannerSettings && Object.keys(appBannerSettings).length != 0
)

const appUrl = computed(() => {
return buildUrl(router, `/f/${props.fileId}`)
.toString()
.replace('https', appBannerSettings.appScheme)
.replace('https', currentTheme.value.appBanner?.appScheme)
})

const close = () => {
Expand All @@ -72,9 +80,9 @@ export default defineComponent({
return {
appUrl,
close,
isVisible,
appBannerSettings,
isAppBannerAvailable
currentTheme,
isAppBannerAvailable,
isVisible
}
}
})
Expand Down
14 changes: 7 additions & 7 deletions packages/web-pkg/src/composables/appDefaults/useDocumentTitle.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
import { storeToRefs } from 'pinia'
import { watch, Ref, unref } from 'vue'
import { useStore } from '../store'
import { Store } from 'vuex'
import { useEventBus } from '../eventBus'
import { useThemeStore } from '../piniaStores'
import { EventBus } from '../../services'

interface DocumentTitleOptions {
titleSegments: Ref<string[]>
store?: Store<any>
eventBus?: EventBus
}

export function useDocumentTitle({ titleSegments, store, eventBus }: DocumentTitleOptions): void {
store = store || useStore()
export function useDocumentTitle({ titleSegments, eventBus }: DocumentTitleOptions): void {
const themeStore = useThemeStore()
const { currentTheme } = storeToRefs(themeStore)

eventBus = eventBus || useEventBus()

watch(
Expand All @@ -20,10 +21,9 @@ export function useDocumentTitle({ titleSegments, store, eventBus }: DocumentTit
const titleSegments = unref(newTitleSegments)

const glue = ' - '
const generalName = store.getters['configuration'].currentTheme.general.name
const payload = {
shortDocumentTitle: titleSegments.join(glue),
fullDocumentTitle: [...titleSegments, generalName].join(glue)
fullDocumentTitle: [...titleSegments, currentTheme.value.common.name].join(glue)
}

eventBus.publish('runtime.documentTitle.changed', payload)
Expand Down
1 change: 1 addition & 0 deletions packages/web-pkg/src/composables/piniaStores/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export * from './extensionRegistry'
export * from './theme'
Loading