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.

Sign up for GitHub

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

Jump to bottom

Redesign main layout followup #6228

Merged
merged 19 commits into from Jan 13, 2022
Merged

Redesign main layout followup #6228

merged 19 commits into from Jan 13, 2022

Conversation

lookacat
Copy link
Contributor

@lookacat lookacat commented Jan 9, 2022 •
edited by kulmann

Description

See #6203

Things done with this PR:

  • Fix tooltip reactivity
  • Reimplement highlighting animation
  • Small UI fixes / tweaks

Related Issue

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 added
  • Acceptance tests added
  • Documentation ticket raised:

@update-docs
Copy link

update-docs bot commented Jan 9, 2022

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-followup branch 4 times, most recently from 7ee248a to 7f85dd1 Compare January 9, 2022 17:00
@pascalwengerter pascalwengerter mentioned this pull request Jan 13, 2022
Closed
25 tasks
@kulmann kulmann mentioned this pull request Jan 13, 2022
Closed
@kulmann kulmann mentioned this pull request Jan 13, 2022
Merged
9 tasks
@kulmann kulmann marked this pull request as ready for review January 13, 2022 21:40
@kulmann kulmann requested a review from fschade as a code owner January 13, 2022 21:40
pascalwengerter
pascalwengerter approved these changes Jan 13, 2022
View reviewed changes
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.

Nice! Minor nitpick regarding compatibility/semver

packages/web-runtime/src/components/SidebarNav/SidebarNav.vue
:target="link.route.path"
:active="link.active"
:icon="link.icon || link.iconMaterial"
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this (in theory) a breaking change for extension developer? Pretty sure we'll have to change iconMaterial from accounts&settings in oCIS, e.g.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, true, I'll revert it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nono it's good actually, the iconMaterial doesn't make sense to keep and we're doing breaking changes already 👌🏽

@pascalwengerter
Copy link
Contributor

I'll push fixed unit tests in a second and then squash-merge if CI turns green

@kulmann
Copy link
Member

kulmann commented Jan 13, 2022

Squash-merge is good, but I'll also have another ods update in a moment.

@kulmann
Copy link
Member

kulmann commented Jan 13, 2022

Ok, ready for merge (when CI turns green) from my side now.

@ownclouders
Copy link
Contributor

Results for e2e-tests oCIS https://drone.owncloud.com/owncloud/web/21883/11/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-13-05-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-13-38-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-14-11-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-14-43-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-15-16-Alice.zip

@ownclouders
Copy link
Contributor

Results for e2e-tests oC10 https://drone.owncloud.com/owncloud/web/21883/10/1

💥 To see the trace, please open the link in the console ...

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-13-48-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-14-22-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-14-56-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-15-30-Alice.zip

npx playwright show-trace https://cache.owncloud.com/owncloud/web/tracing/21883/2022-1-13-10-16-03-Alice.zip

fschade
fschade approved these changes Jan 13, 2022
View reviewed changes
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.

LGTM 👍 once ci is Green

@kulmann
Copy link
Member

kulmann commented Jan 13, 2022

z-index on app switcher or sidebar nav is broken, app switcher lies behind sidebar nav... let me check

@ownclouders
Copy link
Contributor

Results for oCISSharingAndUpload https://drone.owncloud.com/owncloud/web/21884/63/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10SharingFilePerm https://drone.owncloud.com/owncloud/web/21884/25/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@sonarcloud
Copy link

sonarcloud bot commented Jan 13, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

32.7% 32.7% Coverage
0.0% 0.0% Duplication

@ownclouders
Copy link
Contributor

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

  • webUISharingExternalToRoot/federationSharing.feature:378

@kulmann kulmann merged commit f9a2798 into master Jan 13, 2022
@delete-merged-branch delete-merged-branch bot deleted the redesign-followup branch January 13, 2022 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann approved these changes

@pascalwengerter pascalwengerter pascalwengerter approved these changes

@fschade fschade fschade approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Redesign finetuning: Icons + white bar Redesign Follow-up redesign main layout
5 participants
@lookacat @pascalwengerter @kulmann @ownclouders @fschade