Skip to content

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

Merged
juliusknorr merged 30 commits into
masterfrom
poc/redesign
Sep 1, 2022
Merged

⚗️ Proof of concept: Redesign app-content#33568
juliusknorr merged 30 commits into
masterfrom
poc/redesign

Conversation

@CarlSchwan

@CarlSchwanCarlSchwan commented Aug 16, 2022•
edited by juliusknorr
Loading

Copy link
Copy Markdown
Member

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

@CarlSchwanCarlSchwan self-assigned this Aug 16, 2022
juliusknorr
juliusknorr reviewed Aug 16, 2022
View reviewed changes
Comment threadcore/css/apps.scss Outdated
@jancborchardtjancborchardt added design Design, UI, UX, etc. enhancement labels Aug 17, 2022
jancborchardt
jancborchardt reviewed Aug 17, 2022
View reviewed changes
Comment threadcore/css/apps.scss Outdated
skjnldsv
skjnldsv requested changes Aug 17, 2022
View reviewed changes
Comment threadcore/css/apps.scss Outdated
@AndyScherzingerAndyScherzinger changed the title Proof of concept: Redesign app-content⚗️ Proof of concept: Redesign app-contentAug 18, 2022
@juliusknorr
juliusknorrforce-pushed the poc/redesign branch 2 times, most recently from 30d180c to 47a2534CompareAugust 23, 2022 10:01
@juliusknorrjuliusknorr mentioned this pull request Aug 23, 2022
Merged
1 task
@nickvergessennickvergessen 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.

Sign in to view
@juliusknorr

This comment was marked as duplicate.

Sign in to view
@juliusknorr
juliusknorrforce-pushed the poc/redesign branch 2 times, most recently from 013d926 to fe5223dCompareAugust 27, 2022 09:46
github-advanced-security[bot]
github-advanced-securityAI found potential problems Aug 27, 2022
View reviewed changes
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
Comment threadapps/theming/lib/Themes/DefaultTheme.php Fixed
skjnldsv
skjnldsv reviewed Aug 28, 2022
View reviewed changes
Comment threadapps/files/src/views/Sidebar.vue
@juliusknorrjuliusknorr mentioned this pull request Aug 28, 2022
Merged
6 tasks
juliusknorr
juliusknorr reviewed Aug 28, 2022
View reviewed changes
Comment threadapps/theming/lib/Themes/DefaultTheme.php Outdated
@jancborchardt

jancborchardt commented Aug 29, 2022

Copy link
Copy Markdown
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.

juliusknorrand others added 4 commits September 1, 2022 14:15
@juliusknorr
Adjust theming acceptance tests
6dac33e
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Fix fallback css variables
3147585
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@danxuliu@juliusknorr
Wait for the new user form to be visible
76b0947
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>
@danxuliu@juliusknorr
Maximize browser window before running the tests
9292523
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>
This was referenced Sep 1, 2022
Merged
Merged
@danxuliu

danxuliu commented Sep 1, 2022

Copy link
Copy Markdown
Member

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

@juliusknorr
Bump to @nextcloud/vue@7.0.0-beta.0
4ce02c9
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr
Bump bundles
5219992
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorrjuliusknorr mentioned this pull request Sep 1, 2022
Merged
@juliusknorr
Bump @nextcloud/calendar-availability-vue to 0.5.0-beta.2
00b3089
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr

juliusknorr commented Sep 1, 2022

Copy link
Copy Markdown
Member

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

@danxuliu

danxuliu commented Sep 1, 2022

Copy link
Copy Markdown
Member

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

juliusknorr commented Sep 1, 2022

Copy link
Copy Markdown
Member

Thanks for checking that. All other failures seem unrelated.

@juliusknorr
juliusknorr merged commit 12e7f41 into masterSep 1, 2022
@juliusknorr
juliusknorr deleted the poc/redesign branch September 1, 2022 15:03
@juliusknorr

juliusknorr commented Sep 1, 2022

Copy link
Copy Markdown
Member

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

This was referenced Sep 1, 2022
Open
Closed
Closed
Closed
Closed
Closed
@skjnldsv

skjnldsv commented Sep 1, 2022•
edited
Loading

Copy link
Copy Markdown
Member

When opening a photo in Photos

Sidebar.vue:464Uncaught(inpromise) TypeError: Cannotreadpropertiesofnull(reading'classList')ato.setFullScreenMode(Sidebar.vue:464)ata.beforeOpen(Viewer.vue:449)atViewer.vue:460atu(runtime.js:63)atGenerator._invoke(runtime.js:294)atGenerator.next(runtime.js:119)atjg(Pencil.vue?12b8:19)ata(Pencil.vue?12b8:19)atPencil.vue?12b8:19atnewPromise(<anonymous>)

EDIT: FIX in #33814

This was referenced Sep 1, 2022
Closed
Closed
Closed
@juliusknorr

juliusknorr commented Sep 1, 2022

Copy link
Copy Markdown
Member

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

https://github.com/nextcloud/server/labels/ui-refresh-feedback

@juliusknorrjuliusknorr mentioned this pull request Sep 2, 2022
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@juliusknorrjuliusknorrjuliusknorr approved these changes
@PVince81PVince81PVince81 approved these changes
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@raimund-schluesslerraimund-schluesslerAwaiting requested review from raimund-schluessler
@jotoerijotoeriAwaiting requested review from jotoeri
@tacructacrucAwaiting requested review from tacruc
@tcitworldtcitworldAwaiting requested review from tcitworld
@RelloRelloAwaiting requested review from Rello
@jancborchardtjancborchardtAwaiting requested review from jancborchardt
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@skjnldsvskjnldsvAwaiting requested review from skjnldsvskjnldsv is a code owner
@jospoortvlietjospoortvlietAwaiting requested review from jospoortvliet
@PytalPytalAwaiting requested review from Pytal

Assignees

@CarlSchwanCarlSchwan

Labels

4. to releaseReady to be released and/or waiting for tests to finishdesignDesign, UI, UX, etc.enhancement

Projects

None yet

Milestone

Nextcloud 25

Development

Successfully merging this pull request may close these issues.

13 participants

@CarlSchwan@juliusknorr@jancborchardt@Rello@PVince81@ChristophWurst@tacruc@blizzz@danxuliu@skjnldsv@jospoortvliet@github-advanced-security@nickvergessen