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

Reduce indent for app-navigation-caption #34082

Closed
wants to merge 2 commits into from

Conversation

meichthys
Copy link

No description provided.

Copy link
Author

@meichthys meichthys left a comment

Choose a reason for hiding this comment

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

Slight improvement to the settings navigation sidebar.

Prior to change:
image

After change:
image

@szaimen szaimen added this to the Nextcloud 25 milestone Sep 14, 2022
@szaimen szaimen requested review from a team, PVince81 and Pytal and removed request for a team September 14, 2022 15:00
@jancborchardt
Copy link
Member

I think it always looks better if the text is aligned, and in the case of these headings it gives them the necessary breathing room to highlight them more too.

But what do you think @nimishavijay @marcoambrosini?

@nimishavijay
Copy link
Member

Agreed with @jancborchardt, we always try to make sure all text is left aligned.

@marcoambrosini
Copy link
Member

marcoambrosini commented Sep 15, 2022

I prefer left-alignment tbh. I don't agree with the "stand-out" argument you make @jancborchardt. I think it actually works the other way around: if it's left aligned, the fact that the caption "breaks" an otherwise homogenous column of text makes it stand out more.
That said, I'll be at peace with both solutions :)

edit: just saw your comment @nimishavijay. Now I'm not sure what @jancborchardt meant 😅

@meichthys
Copy link
Author

meichthys commented Sep 15, 2022

@jancborchardt, @marcoambrosini, and @nimishavijay,

Perhaps we can increase the font-size to 20px to follow the official nextcloud typography guidelines?

Current:
image

Following Typography guidelines:
image

@skjnldsv skjnldsv mentioned this pull request Sep 15, 2022
This was referenced Sep 20, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@jancborchardt
Copy link
Member

Alright, @meichthys then let’s get in your improvement – however, how come it is 15px? We should work with multiples of 4px which is our baseline. So you could use e.g.: calc(var(--default-grid-baseline, 4px) * 4) for 16px.

Regarding the increased font-size: That is only for headings, but not for simple captions like this. :) The focus should be on the content, the entries themselves.

Signed-off-by: meichthys <10717998+meichthys@users.noreply.github.com>
Signed-off-by: MeIchthys <10717998+meichthys@users.noreply.github.com>
Signed-off-by: meichthys <10717998+meichthys@users.noreply.github.com>
@meichthys
Copy link
Author

meichthys commented Oct 12, 2022

@jancborchardt we should be good to go.
IMO, the navigation captions would look better if they were headings, but I see your point about focusing on the content instead 👍

@blizzz blizzz mentioned this pull request Feb 1, 2023
@skjnldsv skjnldsv mentioned this pull request Feb 23, 2023
@blizzz blizzz mentioned this pull request Mar 7, 2023
@blizzz blizzz modified the milestones: Nextcloud 26, Nextcloud 27 Mar 9, 2023
@skjnldsv skjnldsv mentioned this pull request May 3, 2023
This was referenced May 9, 2023
@skjnldsv skjnldsv modified the milestones: Nextcloud 27, Nextcloud 28 May 10, 2023
@szaimen
Copy link
Contributor

szaimen commented May 10, 2023

@meichthys can you please rebase, run npm ci and npm run build and commit the files here? Thanks a lot!

@szaimen szaimen added 2. developing Work in progress and removed 3. to review Waiting for reviews labels May 10, 2023
@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
This was referenced Mar 12, 2024
This was referenced Mar 20, 2024
@skjnldsv skjnldsv mentioned this pull request Mar 28, 2024
81 tasks
@skjnldsv skjnldsv modified the milestones: Nextcloud 29, Nextcloud 30 Mar 28, 2024
@skjnldsv
Copy link
Member

skjnldsv commented May 2, 2024

Closing this pull request due to lack of recent activity and updates. We appreciate your contribution and encourage you to reopen or provide further updates if necessary. ☺️
Our aim is to keep the project moving forward with active collaboration. Thank you for your understanding and continued support! 🙏

@skjnldsv skjnldsv added the stale Ticket or PR with no recent activity label May 2, 2024
@skjnldsv skjnldsv closed this May 2, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress stale Ticket or PR with no recent activity ui-refresh-feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants