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

⚗️ Proof of concept: Redesign app-content #33568

Merged
merged 30 commits into from
Sep 1, 2022
Merged

Conversation

CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Aug 16, 2022

ToDo

  • Mobile should have top border radius of 14 px
  • Decide on how to handle the Safari iOS issues
  • Expose dashboard image to all pages
  • Keep existing full background image on dashboard and fix scrolling
  • BUG: Fix user menu (top right overlap in viewer sidebar)
  • BUG: Files - app navigation settings button should not have a background
  • BUG: Recent/share file list control header sticky position is off
  • BUG: External storage file list to wide and causes horizontal scrolling
    Issues with moving body scroll to a scrollable element for main content
  • Accessibility
  • Performance especially on mobile
    • May need will-change css attribute for extending cpu rendering (didn't seem necessary when debugging with performance dev tools in chromium/firefox)
    • Testing didn't show any noticeable issues on Android/iOS also with older android device
  • Issues with mobile (especially iOS) where the height is calculated differently -> Only an issue on iOS Safari and seems acceptable as discussed with @jancborchardt

Follow ups

Test matrix for non-vue or server apps

  • Files
  • Public folder shares
  • Public file shares
  • Error/public pages (e.g. 2FA, login flow)
  • Activity
  • Profile page
  • Settings
  • Help => cannot test, need a server build to see contents
  • Apps page 🚫
Screenshots

image

core/css/apps.scss Outdated Show resolved Hide resolved
@jancborchardt jancborchardt added design Design, UI, UX, etc. enhancement labels Aug 17, 2022
core/css/apps.scss Outdated Show resolved Hide resolved
core/css/apps.scss Outdated Show resolved Hide resolved
@AndyScherzinger AndyScherzinger changed the title Proof of concept: Redesign app-content ⚗️ Proof of concept: Redesign app-content Aug 18, 2022
@juliusknorr juliusknorr force-pushed the poc/redesign branch 2 times, most recently from 30d180c to 47a2534 Compare August 23, 2022 10:01
@nickvergessen nickvergessen added 1. to develop Accepted and waiting to be taken care of and removed 1 - to develop labels Aug 25, 2022
@juliusknorr

This comment was marked as duplicate.

@juliusknorr

This comment was marked as duplicate.

@juliusknorr juliusknorr force-pushed the poc/redesign branch 2 times, most recently from 013d926 to fe5223d Compare August 27, 2022 09:46
@jancborchardt
Copy link
Member

FYI @raimund-schluessler @jotoeri @tcitworld @tacruc @Rello since you are very active in the Vue components and/or in app development, it would be awesome if you can test this! :) Then we can make sure to adjust stuff that doesn’t work yet.

juliusknorr and others added 4 commits September 1, 2022 14:15
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Before it was checked if the new user form was visible, but it was not
waited for it. It seems that it can happen that the new user form is in
the DOM, and therefore found, but not visible yet when the tests run,
which caused them to (randomly) fail. Due to that now it is explicitly
waited until it is visible, rather than assuming that it is visible as
soon as it appears in the DOM.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
By the default the browser window is not maximized, but opened with a
size of 1050x978px. In the Files app, when the navigation bar and the
side bar are both open, with the previous design that width caused the
file name to be very very narrow, but still clickable. However, with the
updated design the file name is too narrow and no longer clickable,
which breaks several acceptance tests that descend into subfolders. To
solve that now the browser window is maximized before running the tests,
which makes the window wide enough (1360px) to show the file name and
make it clickable.

This commit also removes a step to close the sidebar that was recently
added to address the problem mentioned above in a previous pull request.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu
Copy link
Member

danxuliu commented Sep 1, 2022

app-files-sharing-link.feature:14 passes when run locally, so failure should be unrelated.

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

Seems one additional failure at features/app-files-sharing.feature:422 but passes locally. @danxuliu Shall we also consider that unrelated?

@danxuliu
Copy link
Member

danxuliu commented Sep 1, 2022

Seems one additional failure at features/app-files-sharing.feature:422 but passes locally. @danxuliu Shall we also consider that unrelated?

Yes, I guess that the problem is that sometimes the share menu is not properly opened and therefore the item can not be found (which is probably the same underlying issue as in app-files-sharing-link.feature:14), but it should not be related to the changes in this pull request.

@juliusknorr
Copy link
Member

Thanks for checking that. All other failures seem unrelated.

@juliusknorr juliusknorr merged commit 12e7f41 into master Sep 1, 2022
@juliusknorr juliusknorr deleted the poc/redesign branch September 1, 2022 15:03
@juliusknorr
Copy link
Member

Will take care of filing the reportings into follow up tickets.

@skjnldsv
Copy link
Member

skjnldsv commented Sep 1, 2022

When opening a photo in Photos

Sidebar.vue:464 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'classList')
    at o.setFullScreenMode (Sidebar.vue:464)
    at a.beforeOpen (Viewer.vue:449)
    at Viewer.vue:460
    at u (runtime.js:63)
    at Generator._invoke (runtime.js:294)
    at Generator.next (runtime.js:119)
    at jg (Pencil.vue?12b8:19)
    at a (Pencil.vue?12b8:19)
    at Pencil.vue?12b8:19
    at new Promise (<anonymous>)

EDIT: FIX in #33814

@juliusknorr
Copy link
Member

Filed all inline reportings with the ui-refresh-feedback label so we get a better overview:

ui-refresh-feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish design Design, UI, UX, etc. enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.