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 app container border radius #44786

Merged
merged 3 commits into from
May 7, 2024
Merged

Conversation

marcoambrosini
Copy link
Member

Screenshot 2024-04-11 at 10 23 56

Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Will hard-coding introduce any new regressions for designs around the server? If there is a specific spot that we want to handle, should we not change the radius there instead? I am just curious for the reason, if its something that the designers have decided then I am fine with this :)

@marcoambrosini
Copy link
Member Author

Hi @emoral435, this is part of a series of changes that the design team is making. Part of it is dialing back the border radii of some elements. So we want this change to be in all Nextcloud not just in a few places.

Copy link
Member

@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.

@marcoambrosini what @emoral435 is saying though is that setting it to 12px only here is a bit strange. Do we never use 12px as radius anywhere else? Shouldn’t it be using a variable, maybe even just 3*default-grid-baseline?

Copy link
Contributor

@emoral435 emoral435 left a comment

Choose a reason for hiding this comment

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

Much nicer :) Thank you!

Copy link
Member

@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.

Looks good! :)

@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
@marcoambrosini marcoambrosini force-pushed the reduce-border-radius branch 2 times, most recently from ddff451 to f8ab2e6 Compare May 6, 2024 13:40
@marcoambrosini marcoambrosini force-pushed the reduce-border-radius branch 2 times, most recently from 687e3f1 to 4824908 Compare May 6, 2024 14:32
Signed-off-by: Marco <marcoambrosini@icloud.com>
@jancborchardt jancborchardt requested review from susnux and artonge and removed request for emoral435 May 7, 2024 09:15
@jancborchardt
Copy link
Member

@nimishavijay @susnux @artonge do you have any further feedback? Otherwise let’s get this in while we are still early in the cycle. :)

@jancborchardt jancborchardt added the design Design, UI, UX, etc. label May 7, 2024
Copy link
Contributor

@susnux susnux 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, if this was discussed by the design team then 👍

@marcoambrosini marcoambrosini merged commit 0af06f6 into master May 7, 2024
104 checks passed
@marcoambrosini marcoambrosini deleted the reduce-border-radius branch May 7, 2024 09:41
@marcoambrosini marcoambrosini mentioned this pull request Jun 10, 2024
5 tasks
@blizzz blizzz mentioned this pull request Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews design Design, UI, UX, etc. enhancement
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants