Custom dashboard background by juliusknorr · Pull Request #22214 · nextcloud/server

juliusknorr · 2020-08-12T08:04:33Z

Follow up to #22143

jancborchardt · 2020-08-16T23:18:55Z

Added some design improvements to the modal. 2 big accessibility issues:

The "Customize" modal is not possible to invoke via keyboard only. The "Customize" button only works on click
The background picker is tabbable, but it’s not possible to change the background. Semantically it would be better if it’s radio buttons instead of divs, as it’s a choice of 1 item.

juliusknorr · 2020-08-17T19:41:27Z

@jancborchardt Feel free to give this another test run in terms of accessibility, that should all work nicely with keyboard navigation now.

There are two things missing now before we can get this merged:

Attribution for all shipped backgrounds - I'm not sure yet how we should do this nicely without having a long list at the bottom
Define which background should use dark and which ones should use white icons in the header (this will be some non-elegant workaround to make theming work together with the images, but better than having black icons on a dark background)

julien-nc · 2020-08-18T12:11:06Z

@jancborchardt As mentioned there the statuses are too close. I could add a margin in the weather status but I think it's better to add it in the dashboard to make sure it's done for all future statuses. What do you think?

Other detail: I set the button background in the weather status but it could be done the same way it's done for user status.

juliusknorr · 2020-08-18T12:53:06Z

@jancborchardt As mentioned there the statuses are too close. I could add a margin in the weather status but I think it's better to add it in the dashboard to make sure it's done for all future statuses. What do you think?

I pushed a commit here to add some spacing.

Other detail: I set the button background in the weather status but it could be done the same way it's done for user status.

I'd say we keep this in the apps, so we stay flexible in what is displayed, some might use a button others maybe just a div without any actions.

julien-nc · 2020-08-18T13:34:06Z

Oh and as a reminder: We should put attributions for

Met.no (link to https://api.met.no/doc/TermsOfService)
Nominatim (link to OSM privacy policy https://wiki.osmfoundation.org/wiki/Privacy_Policy)
OpenTopoData used to get elevation data (link to https://www.opentopodata.org/#public-api)

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Signed-off-by: Julius Härtl <jus@bitgrid.net> update images Signed-off-by: Julius Härtl <jus@bitgrid.net>

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Signed-off-by: Jan C. Borchardt <hey@jancborchardt.net>

Signed-off-by: Julius Härtl <jus@bitgrid.net>

juliusknorr · 2020-08-19T15:10:04Z

apps/dashboard/css/dashboard.scss

@@ -0,0 +1,71 @@
+// Show Dashboard background image beneath header


I admit this is not a nice approach to overwrite the global styles like this, but it is the only way in my oppinion that we can nicely adjust header icons on bright theming colors with light and dark mode from accessibility.

It's fine, we cannot always win on this 🤷
It's also decently straightforward

apps/dashboard/css/dashboard.scss

skjnldsv · 2020-08-19T15:27:04Z

apps/dashboard/css/dashboard.scss

+	}
+}
+
+body.dashboard-dark:not(.dashboard-inverted) {


BEM syntax? 🤷

Suggested change

body.dashboard-dark:not(.dashboard-inverted) {

body.dashboard--dark:not(.dashboard--inverted) {

skjnldsv · 2020-08-19T15:30:36Z

apps/dashboard/src/components/BackgroundSettings.vue

+		<a class="background filepicker"
+			:class="{ active: background === 'custom' }"
+			tabindex="0"
+			@click="pickFile"
+			@keyup.enter="pickFile"
+			@keyup.space="pickFile">
+			{{ t('dashboard', 'Pick from files') }}
+		</a>
+		<a class="background default"
+			tabindex="0"
+			:class="{ 'icon-loading': loading === 'default', active: background === 'default' }"
+			@click="setDefault"
+			@keyup.enter="setDefault"
+			@keyup.space="setDefault">
+			{{ t('dashboard', 'Default images') }}
+		</a>
+		<a class="background color"
+			:class="{ active: background === 'custom' }"
+			tabindex="0"
+			@click="pickColor"
+			@keyup.enter="pickColor"
+			@keyup.space="pickColor">
+			{{ t('dashboard', 'Plain background') }}
+		</a>
+		<a v-for="shippedBackground in shippedBackgrounds"
+			:key="shippedBackground.name"
+			v-tooltip="shippedBackground.details.attribution"
+			:class="{ 'icon-loading': loading === shippedBackground.name, active: background === shippedBackground.name }"
+			tabindex="0"
+			class="background"
+			:style="{ 'background-image': 'url(' + shippedBackground.preview + ')' }"
+			@click="setShipped(shippedBackground.name)"
+			@keyup.enter="setShipped(shippedBackground.name)"
+			@keyup.space="setShipped(shippedBackground.name)" />


Why links instead of buttons?
Buttons already have dedicated click/enter/space handlers :)

To be fully correct, I'd say it should be a list of radio buttons, no? That way only one can be selected and we don't need to do that handling manually.

skjnldsv · 2020-08-19T15:31:37Z

apps/dashboard/src/components/BackgroundSettings.vue

+			const background = data.type === 'custom' || data.type === 'default' ? data.type : data.value
+			this.backgroundImage = getBackgroundUrl(background, data.version)
+			if (data.type === 'color') {
+				this.$emit('updateBackground', data)


Suggested change

this.$emit('updateBackground', data)

this.$emit('update:background', data)

sync modifier ?

skjnldsv · 2020-08-19T15:31:48Z

apps/dashboard/src/components/BackgroundSettings.vue

+			}
+			const image = new Image()
+			image.onload = () => {
+				this.$emit('updateBackground', data)


Suggested change

this.$emit('updateBackground', data)

this.$emit('update:background', data)

sync modifier ?

faily-bot · 2020-08-19T16:16:46Z

🤖 beep boop beep 🤖

Here are the logs for the failed build:

Status of 31989: failure

acceptance-app-files

tests/acceptance/features/app-files.feature:262

Show full log

  Scenario: unmarking a file as favorite causes the file list to be sorted again                          # /drone/src/tests/acceptance/features/app-files.feature:262
    Given I am logged in                                                                                  # LoginPageContext::iAmLoggedIn()
    And I create a new folder named "A name alphabetically lower than welcome.txt"                        # FileListContext::iCreateANewFolderNamed()
    And I see that "A name alphabetically lower than welcome.txt" precedes "welcome.txt" in the file list # FileListContext::iSeeThatPrecedesInTheFileList()
    And I close the details view                                                                          # FilesAppContext::iCloseTheDetailsView()
    And I see that the details view is closed                                                             # FilesAppContext::iSeeThatTheDetailsViewIsClosed()
      │ Details view in Files app visibility could not be got
      │ Exception message: Element not found with xpath, (//html//*[@id="app-sidebar" or contains(@class, 'app-sidebar')])[1]
      │ 
      │ Unable to locate element: {"method":"xpath","selector":"(//html//*[@id=\"app-sidebar\" or contains(@class, 'app-sidebar')])[1]"}
      │ For documentation on this error, please visit: http://seleniumhq.org/exceptions/no_such_element.html
      │ Build info: version: '2.53.1', revision: 'a36b8b1', time: '2016-06-30 17:37:03'
      │ System info: host: '528d8bb6069c', ip: '172.18.0.2', os.name: 'Linux', os.arch: 'amd64', os.version: '4.15.0-88-generic', java.version: '1.8.0_91'
      │ Driver info: driver.version: unknown
      │ Trying again
      │ 
    And I mark "welcome.txt" as favorite                                                                  # FileListContext::iMarkAsFavorite()
    And I see that "welcome.txt" is marked as favorite                                                    # FileListContext::iSeeThatIsMarkedAsFavorite()
    And I see that "welcome.txt" precedes "A name alphabetically lower than welcome.txt" in the file list # FileListContext::iSeeThatPrecedesInTheFileList()
    When I unmark "welcome.txt" as favorite                                                               # FileListContext::iUnmarkAsFavorite()
    Then I see that "welcome.txt" is not marked as favorite                                               # FileListContext::iSeeThatIsNotMarkedAsFavorite()
      Not favorited state icon for file welcome.txt in file list could not be found after 100 seconds (NoSuchElementException)
    And I see that "A name alphabetically lower than welcome.txt" precedes "welcome.txt" in the file list # FileListContext::iSeeThatPrecedesInTheFileList()

acceptance-app-files-sharing

tests/acceptance/features/app-files-sharing.feature:338

Show full log

  Scenario: sharee can revoke create permission from reshare after the sharer disabled it # /drone/src/tests/acceptance/features/app-files-sharing.feature:338
    Given I act as John                                                                   # ActorContext::iActAs()
    And I am logged in as the admin                                                       # LoginPageContext::iAmLoggedInAsTheAdmin()
    And I act as Jane                                                                     # ActorContext::iActAs()
    And I am logged in                                                                    # LoginPageContext::iAmLoggedIn()
    And I act as Jim                                                                      # ActorContext::iActAs()
    And I am logged in as "user1"                                                         # LoginPageContext::iAmLoggedInAs()
    And I act as John                                                                     # ActorContext::iActAs()
    And I create a new folder named "Shared folder"                                       # FileListContext::iCreateANewFolderNamed()
    And I see that the file list contains a file named "Shared folder"                    # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    And I share "Shared folder" with "user0"                                              # FilesAppSharingContext::iShareWith()
    And I see that the file is shared with "user0"                                        # FilesAppSharingContext::iSeeThatTheFileIsSharedWith()
    And I act as Jane                                                                     # ActorContext::iActAs()
    And I open the Files app                                                              # FilesAppContext::iOpenTheFilesApp()
    And I share "Shared folder" with "user1"                                              # FilesAppSharingContext::iShareWith()
    And I see that the file is shared with "user1"                                        # FilesAppSharingContext::iSeeThatTheFileIsSharedWith()
    And I act as John                                                                     # ActorContext::iActAs()
    And I set the share with "user0" as not creatable                                     # FilesAppSharingContext::iSetTheShareWithAsNotCreatable()
    And I see that "user0" can not create in the share                                    # FilesAppSharingContext::iSeeThatCanNotCreateInTheShare()
    And I act as Jim                                                                      # ActorContext::iActAs()
    And I open the Files app                                                              # FilesAppContext::iOpenTheFilesApp()
    And I enter in the folder named "Shared folder"                                       # FileListContext::iEnterInTheFolderNamed()
    And I create a new folder named "Subfolder"                                           # FileListContext::iCreateANewFolderNamed()
    And I see that the file list contains a file named "Subfolder"                        # FileListContext::iSeeThatTheFileListContainsAFileNamed()
    When I act as Jane                                                                    # ActorContext::iActAs()
    And I open the Files app                                                              # FilesAppContext::iOpenTheFilesApp()
    And I open the details view for "Shared folder"                                       # FileListContext::iOpenTheDetailsViewFor()
    And I see that the details view is open                                               # FilesAppContext::iSeeThatTheDetailsViewIsOpen()
    And I open the "Sharing" tab in the details view                                      # FilesAppContext::iOpenTheTabInTheDetailsView()
    And I see that the "Sharing" tab in the details view is eventually loaded             # FilesAppContext::iSeeThatTheTabInTheDetailsViewIsEventuallyLoaded()
    And I set the share with "user1" as not creatable                                     # FilesAppSharingContext::iSetTheShareWithAsNotCreatable()
    Then I see that "user1" can not create in the share                                   # FilesAppSharingContext::iSeeThatCanNotCreateInTheShare()
    And I see that "user1" can not be allowed to create in the share                      # FilesAppSharingContext::iSeeThatCanNotBeAllowedToCreateInTheShare()
    And I act as Jim                                                                      # ActorContext::iActAs()
    And I open the Files app                                                              # FilesAppContext::iOpenTheFilesApp()
    And I enter in the folder named "Shared folder"                                       # FileListContext::iEnterInTheFolderNamed()
    And I see that it is not possible to create new files                                 # FileListContext::iSeeThatItIsNotPossibleToCreateNewFiles()
      Failed asserting that true is false.

jancborchardt

Apart from @skjnldsv's code review, design-wise it works nicely. :)

nickvergessen · 2020-08-19T19:34:58Z

Minor issue:
Theming color: #35FFDC
Dark mode: enabled
Background: Pedra azul milky way

Contacts icon is still black somehow?

nickvergessen

🦈 LGTM time to start a follow up or fix the tiny comments from skj here too

apps/dashboard/lib/Controller/DashboardController.php

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

MorrisJobke · 2020-08-19T19:55:48Z

I just added a type hint and an explicit cast from int to string to make Psalm happy. Ready to be merged.

juliusknorr · 2020-08-19T19:57:25Z

Ah now i was to slow, then I'll do a follow up ;)

juliusknorr added the 2. developing Work in progress label Aug 12, 2020

jancborchardt added enhancement feature: dashboard labels Aug 12, 2020

jancborchardt added this to the Nextcloud 20 milestone Aug 12, 2020

juliusknorr force-pushed the dashboard/design-fixes branch 2 times, most recently from 8cea2cc to efb56f8 Compare August 14, 2020 15:22

julien-nc mentioned this pull request Aug 18, 2020

weather status used in dashboard #22124

Merged

3 tasks

juliusknorr force-pushed the dashboard/design-fixes branch from 28e51dc to cb3bd47 Compare August 18, 2020 12:51

jancborchardt self-assigned this Aug 18, 2020

juliusknorr added 2 commits August 19, 2020 13:55

Sticky header background

a3bcf73

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Move customize button to the bottom

1e043d5

Signed-off-by: Julius Härtl <jus@bitgrid.net>

jancborchardt force-pushed the dashboard/design-fixes branch 2 times, most recently from fcd8403 to e1f9322 Compare August 19, 2020 13:55

jancborchardt removed their assignment Aug 19, 2020

juliusknorr force-pushed the dashboard/design-fixes branch from acc0ad0 to 0f0cec8 Compare August 19, 2020 14:55

juliusknorr and others added 10 commits August 19, 2020 17:07

Allow to set a custom background

4ec0afd

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Fix loading as guest

2535e0e

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Implement background reset and proper shipped setting

b49f814

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Push widget metadata to callback

338f09b

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Do not reset background on every save

4118cb0

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Add images

9a90351

Signed-off-by: Julius Härtl <jus@bitgrid.net> update images Signed-off-by: Julius Härtl <jus@bitgrid.net>

Remove old shipped background

8d22d02

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Dashboard: Customize modal design improvements

38afee8

Signed-off-by: Jan C. Borchardt <hey@jancborchardt.net>

Fix keyboard navigation

fa2072e

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Allow to set a primary color background

5661775

Signed-off-by: Julius Härtl <jus@bitgrid.net>

Bump bundles

93e671e

Signed-off-by: Julius Härtl <jus@bitgrid.net>

juliusknorr force-pushed the dashboard/design-fixes branch from 03b958b to 93e671e Compare August 19, 2020 15:07

juliusknorr added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 19, 2020

juliusknorr marked this pull request as ready for review August 19, 2020 15:08

juliusknorr commented Aug 19, 2020

View reviewed changes

juliusknorr requested review from MorrisJobke, jancborchardt, julien-nc, nickvergessen, rullzer and skjnldsv and removed request for jancborchardt August 19, 2020 15:10

skjnldsv reviewed Aug 19, 2020

View reviewed changes

apps/dashboard/css/dashboard.scss Show resolved Hide resolved

skjnldsv reviewed Aug 19, 2020

View reviewed changes

jancborchardt approved these changes Aug 19, 2020

View reviewed changes

nickvergessen approved these changes Aug 19, 2020

View reviewed changes

MorrisJobke reviewed Aug 19, 2020

View reviewed changes

apps/dashboard/lib/Controller/DashboardController.php Outdated Show resolved Hide resolved

Fix psalm warnings

72df65c

Signed-off-by: Morris Jobke <hey@morrisjobke.de>

MorrisJobke merged commit d3a53d6 into master Aug 19, 2020

MorrisJobke deleted the dashboard/design-fixes branch August 19, 2020 19:55

juliusknorr mentioned this pull request Aug 19, 2020

Dashboard code cleanup #22307

Merged

		@@ -0,0 +1,71 @@
		// Show Dashboard background image beneath header

	body.dashboard-dark:not(.dashboard-inverted) {
	body.dashboard--dark:not(.dashboard--inverted) {

	this.$emit('updateBackground', data)
	this.$emit('update:background', data)

Uh oh!

Conversation

juliusknorr commented Aug 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jancborchardt commented Aug 16, 2020

Uh oh!

juliusknorr commented Aug 17, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

julien-nc commented Aug 18, 2020

Uh oh!

juliusknorr commented Aug 18, 2020

Uh oh!

julien-nc commented Aug 18, 2020

Uh oh!

juliusknorr Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

skjnldsv Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

skjnldsv Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

skjnldsv Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

jancborchardt Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

skjnldsv Aug 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

skjnldsv Aug 19, 2020

Choose a reason for hiding this comment

Uh oh!

faily-bot bot commented Aug 19, 2020

Status of 31989: failure

acceptance-app-files

acceptance-app-files-sharing

Uh oh!

jancborchardt left a comment

Choose a reason for hiding this comment

Uh oh!

nickvergessen commented Aug 19, 2020

Uh oh!

nickvergessen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MorrisJobke commented Aug 19, 2020

Uh oh!

juliusknorr commented Aug 19, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

juliusknorr commented Aug 12, 2020 •

edited

Loading

juliusknorr commented Aug 17, 2020 •

edited

Loading

skjnldsv Aug 19, 2020 •

edited

Loading