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

[full-ci] Redesign main layout #6086

Merged
merged 15 commits into from
Dec 30, 2021
Merged

[full-ci] Redesign main layout #6086

merged 15 commits into from
Dec 30, 2021

Conversation

lookacat
Copy link
Contributor

@lookacat lookacat commented Dec 2, 2021

Description

See #6036

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests updated
  • Acceptance tests updated

@update-docs
Copy link

update-docs bot commented Dec 2, 2021

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@lookacat lookacat force-pushed the redesign-topbar branch 2 times, most recently from 68769b4 to 1f16c66 Compare December 3, 2021 14:01
Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

@paulnebr could you move the application switcher out of the left sidebar into the topbar component within the runtime? Otherwise the user would run into issues both on mobile and when the rendered app doesn't have any navigation items :)

@lookacat
Copy link
Contributor Author

lookacat commented Dec 6, 2021

@pascalwengerter i'll try

@lookacat lookacat force-pushed the redesign-topbar branch 2 times, most recently from 6c48ba1 to 96bee10 Compare December 7, 2021 12:26
@lookacat lookacat force-pushed the redesign-topbar branch 2 times, most recently from 1a83c24 to 565e979 Compare December 8, 2021 14:38
@lookacat lookacat changed the title Redesign main layout [full-ci] Redesign main layout Dec 9, 2021
@lookacat lookacat force-pushed the redesign-topbar branch 9 times, most recently from 25abe17 to 7206cd0 Compare December 9, 2021 10:11
packages/web-runtime/src/App.vue Outdated Show resolved Hide resolved
@lookacat lookacat marked this pull request as ready for review December 10, 2021 09:10
lookacat and others added 12 commits December 28, 2021 17:36
WIP

WIP
fix top bar right side actions

fix topbar & linting

fix unittests

remove inline style
author Paul Neubauer <paulneubauer@live.de> 1639010639 +0100
committer pwengerter <pascal@wengerter.info> 1640347147 +0100

parent c25b33f
author Paul Neubauer <paulneubauer@live.de> 1639010639 +0100
committer pwengerter <pascal@wengerter.info> 1640347043 +0100

move sidebar navigation from ods to web

fix animation style issue

fix initial bug

add newline to yarn lock

fix unittests

fix sidebar naming

add changelog

update snapshot tests

enable smoke tests again

fix rebase gone

fix fav id

fix favourites id

Check deeper in left navbar for navItem text in acceptance tests
@fschade fschade added the Status:Needs-Review Needs review from a maintainer label Dec 29, 2021
Copy link
Collaborator

@fschade fschade left a comment

Choose a reason for hiding this comment

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

some tiny findings, beside that:

  • top left rounded corner has a white artifact border
  • overflow of main area has a margin around it
  • a lot of button inverse hard coded, could be problematic if the user has a bright theme

packages/web-app-files/src/App.vue Show resolved Hide resolved
image-rendering: auto;
image-rendering: crisp-edges;
image-rendering: pixelated;
image-rendering: -webkit-optimize-contrast;
Copy link
Collaborator

@fschade fschade Dec 29, 2021

Choose a reason for hiding this comment

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

hmmm, do we need to add -webkit-optimize-contrast manually or can't post-css help here?

Comment on lines 21 to 22
const navigation = state.navigation
return {
Copy link
Collaborator

@fschade fschade Dec 29, 2021

Choose a reason for hiding this comment

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

can't we use spreading in both cases?

}

return 'fade'
return this.sidebarNavItems.length && this.windowWidth >= 1200
Copy link
Collaborator

@fschade fschade Dec 29, 2021

Choose a reason for hiding this comment

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

perfect candidate for a shared helper isScreenM, isScreenXL, .... Not topic of this pr

packages/web-runtime/src/App.vue Show resolved Hide resolved
tests/e2e/cucumber/step_definitions/app.files.ts Outdated Show resolved Hide resolved
@ownclouders
Copy link
Contributor

Results for oC10SharingExternalRoot https://drone.owncloud.com/owncloud/web/21440/41/1
The following scenarios passed on retry:

  • webUISharingExternalToRoot/federationSharing.feature:378

@ownclouders
Copy link
Contributor

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/21440/15/1
The following scenarios passed on retry:

  • webUISharingAcceptShares/acceptShares.feature:111

@fschade fschade self-requested a review December 30, 2021 10:34
Copy link
Collaborator

@fschade fschade left a comment

Choose a reason for hiding this comment

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

GO 👍 , changes get resolved in separat PR

@@ -9,7 +9,7 @@
:aria-label="ariaLabel"
aria-describedby="oc-feedback-link-description"
>
<oc-icon name="feedback" />
<oc-icon name="feedback" variation="inverse" />
Copy link
Member

Choose a reason for hiding this comment

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

Will not be inverse anymore as soon as the redesign is complete. Ok as intermediate solution.

@sonarcloud
Copy link

sonarcloud bot commented Dec 30, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug B 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 3 Code Smells

69.2% 69.2% Coverage
0.0% 0.0% Duplication

@kulmann kulmann merged commit 3d56800 into master Dec 30, 2021
@delete-merged-branch delete-merged-branch bot deleted the redesign-topbar branch December 30, 2021 12:14
@pascalwengerter pascalwengerter mentioned this pull request Dec 31, 2021
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants