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 sidebar buttons pixel perfect #3518

Merged
merged 2 commits into from
May 20, 2024
Merged

Make sidebar buttons pixel perfect #3518

merged 2 commits into from
May 20, 2024

Conversation

CarlSchwan
Copy link
Contributor

Tested in talk, calendar and files

Before After
image image

@CarlSchwan CarlSchwan added the 3. to review Waiting for reviews label Nov 25, 2022
@CarlSchwan CarlSchwan added this to the 8.0.0 milestone Nov 25, 2022
@CarlSchwan CarlSchwan requested review from marcoambrosini and a team November 25, 2022 11:07
@CarlSchwan CarlSchwan self-assigned this Nov 25, 2022
@CarlSchwan CarlSchwan requested review from artonge and julien-nc and removed request for a team November 25, 2022 11:07
@skjnldsv skjnldsv added the design Design, UX, interface and interaction design label Nov 25, 2022
@skjnldsv
Copy link
Contributor

I think this should be a css variable so that we can easily adjust the spacing of the inner app-content too

How is it looking in photos then?

@CarlSchwan
Copy link
Contributor Author

This is how photo looks like:

image

marcoambrosini
marcoambrosini approved these changes Nov 28, 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.

Oh yes!

@skjnldsv
Copy link
Contributor

Cypress needs an update

@skjnldsv
Copy link
Contributor

This is how photo looks like:

You have viewer opened on your screenshot.
There is a header in photos too, i'd like to see the alignment :)
Capture d’écran_2022-11-29_08-51-20

@CarlSchwan
Copy link
Contributor Author

With photo
image

Looks alligned to me ;)

It took me a while how to figure out how to make the sidebar appear...

@CarlSchwan CarlSchwan force-pushed the fix/pixel-perfect-sidebar-button branch from 0c9f794 to 6fd6144 Compare December 1, 2022 11:27
@CarlSchwan
Copy link
Contributor Author

cypress updated :)

@skjnldsv
Copy link
Contributor

skjnldsv commented Dec 1, 2022

I think this should be a css variable so that we can easily adjust the spacing of the inner app-content too

How is it looking in photos then?

I think this is still valid though 🤷
So we can use the same variable for photos alignments or Talk :)

@marcoambrosini
Copy link
Contributor

there is this atm https://github.com/nextcloud/nextcloud-vue/blob/master/src/components/NcAppContent/NcAppContent.vue#L341

@skjnldsv
Copy link
Contributor

skjnldsv commented Dec 2, 2022

there is this atm master/src/components/NcAppContent/NcAppContent.vue#L341

Then this should be used!
Btw, is this documented anywhere? Are you using it in Talk?

@susnux susnux modified the milestones: 8.0.0, 8.1.0 Nov 8, 2023
@AndyScherzinger AndyScherzinger modified the milestones: 8.1.0, 8.1.1 Nov 14, 2023
@ShGKme ShGKme modified the milestones: 8.10.1, 8.11.0, 8.11.1 Mar 15, 2024
@Antreesy Antreesy modified the milestones: 8.11.1, 8.11.2 Mar 21, 2024
@Antreesy Antreesy modified the milestones: 8.11.2, 8.11.3 Apr 11, 2024
@Antreesy Antreesy modified the milestones: 8.11.3, 8.11.4 May 7, 2024
@susnux susnux modified the milestones: 8.11.4, 8.12.1 May 19, 2024
@susnux susnux force-pushed the fix/pixel-perfect-sidebar-button branch 3 times, most recently from ece2a99 to 12fadcf Compare May 20, 2024 22:39
@CarlSchwan @susnux
fix: Make sidebar buttons pixel perfect
afb171a 
Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Carl Schwan <carl@carlschwan.eu>
@susnux susnux force-pushed the fix/pixel-perfect-sidebar-button branch from 12fadcf to afb171a Compare May 20, 2024 23:04
@susnux
fix(cypress): Update snapshots
e4be190 
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@CarlSchwan CarlSchwan merged commit 40ad0c2 into master May 20, 2024
19 checks passed
ShGKme
ShGKme reviewed May 21, 2024
View reviewed changes
src/components/NcAppSidebar/NcAppSidebar.vue

/*
Sidebar: to be used within #content
app-content will be shrinked properly
*/
.app-sidebar {
--app-sidebar-width: clamp(300px, 27vw, 500px);
--app-sidebar-padding: #{$app-navigation-padding};
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it needed here? It duplicates an existing variable available on the .content

@susnux susnux mentioned this pull request Jun 20, 2024
Merged
1 task
@Antreesy Antreesy modified the milestones: 8.12.1, 8.13.0 Jun 25, 2024
@susnux susnux mentioned this pull request Jul 3, 2024
Closed
4 tasks
This was referenced Aug 9, 2024
Open
Open
This was referenced Aug 19, 2024
Open
Open
This was referenced Sep 6, 2024
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShGKme ShGKme ShGKme left review comments

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@artonge artonge artonge approved these changes

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@julien-nc julien-nc Awaiting requested review from julien-nc

Assignees

@CarlSchwan CarlSchwan

Labels
3. to review Waiting for reviews design Design, UX, interface and interaction design
Projects
None yet
Milestone
8.13.0
Development

Successfully merging this pull request may close these issues.