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

🥛 Make app navigation semi-transparent #3057

Merged
merged 1 commit into from
Aug 26, 2022
Merged

🥛 Make app navigation semi-transparent #3057

merged 1 commit into from
Aug 26, 2022

Conversation

GretaD
Copy link
Contributor

@GretaD GretaD commented Aug 17, 2022 •
edited
Loading

fixes #3047

  • the image is just for testing, it will be added globally on server. I will remove it after review

appnaaav

@GretaD GretaD added the 2. developing Work in progress label Aug 17, 2022
@GretaD GretaD self-assigned this Aug 17, 2022
ChristophWurst
ChristophWurst reviewed Aug 18, 2022
View reviewed changes
Copy link
Contributor

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

Looks good so far

src/components/Content/Content.vue Outdated Show resolved Hide resolved
@GretaD
Copy link
Contributor Author

GretaD commented Aug 18, 2022 •
edited
Loading

the variables names will be changed because i didnt see julius pr: https://github.com/nextcloud/server/pull/33610/files

but that doesnt change the result

jancborchardt
jancborchardt previously requested changes Aug 18, 2022
View reviewed changes
Copy link
Contributor

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

See comments :)

@jancborchardt
Copy link
Contributor

Ah ok got it, then just use @juliushaertl’s variables, my comments are moot. :)

@ChristophWurst

This comment was marked as resolved.

Sign in to view
marcoambrosini
marcoambrosini reviewed Aug 18, 2022
View reviewed changes
Copy link
Contributor

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

this border is not needed anymore https://github.com/nextcloud/nextcloud-vue/blob/0d73e367f29fdad1461e4d2bea6e216342119ddc/src/components/AppNavigation/AppNavigation.vue#L158

@GretaD GretaD marked this pull request as ready for review August 18, 2022 14:35
@GretaD GretaD added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 18, 2022
@AndyScherzinger AndyScherzinger changed the title Make app navigation semi-transparent 🥛 Make app navigation semi-transparent Aug 18, 2022
@ChristophWurst ChristophWurst mentioned this pull request Aug 24, 2022
Merged
@raimund-schluessler raimund-schluessler added the breaking PR that requires a new major version label Aug 24, 2022
@raimund-schluessler raimund-schluessler added this to the 7.0.0 milestone Aug 24, 2022
ChristophWurst
ChristophWurst approved these changes Aug 26, 2022
View reviewed changes
Copy link
Contributor

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

👍 code looks good

@GretaD
Make app navigation semi-transparent
5f59558 
Signed-off-by: greta <gretadoci@gmail.com>
@juliushaertl juliushaertl merged commit 4c483cd into master Aug 26, 2022
@juliushaertl juliushaertl deleted the improve/app-navigation branch August 26, 2022 11:28
@raimund-schluessler raimund-schluessler modified the milestones: 7.0.0, 6.0.0 Aug 26, 2022
@raimund-schluessler raimund-schluessler removed the breaking PR that requires a new major version label Aug 26, 2022
@raimund-schluessler
Copy link
Contributor

@juliushaertl So this will work with NC 24 and below as well, right?

@juliushaertl
Copy link
Contributor

Yes, as we have the fallback to the previous values in place.

@ChristophWurst ChristophWurst mentioned this pull request Aug 26, 2022
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst approved these changes

@juliushaertl juliushaertl juliushaertl approved these changes

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

@jancborchardt jancborchardt Awaiting requested review from jancborchardt

Assignees

@GretaD GretaD

Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.

🥛 Make app navigation semi-transparent
6 participants
@GretaD @jancborchardt @ChristophWurst @raimund-schluessler @juliushaertl @marcoambrosini