Skip to content

The repository list page, pading top and bottom is too small compared as v1.20, increase them so make it looks like before #26761

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

Closed

Conversation

lunny
Copy link
Member

@lunny lunny commented Aug 28, 2023

Before

图片

After

图片

… as v1.20, increase them so make it looks like before
@lunny lunny added topic/ui Change the appearance of the Gitea UI skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Aug 28, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 28, 2023
@wxiaoguang
Copy link
Contributor

Why not just .flex-item { padding: 1em 0; }?

The current approach makes some UI look strange, the first item doesn't have proper padding-top.

@denyskon Do you have ideas for it?

@denyskon
Copy link
Member

It was some sort of issue if you combine flex-item and "ui attached segment", that's why I decided to set the padding only between two items. I'm not at home currently to reproduce it though.....

And about this change: I'm unsure if this would look right everywhere else where we use flex-list... maybe we need to set it only for repo list if we really want 1rem padding there

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Aug 28, 2023

It was some sort of issue if you combine flex-item and "ui attached segment"

Could it be resolved by .ui.attached.segment + .flex-list ?

@silverwind
Copy link
Member

silverwind commented Aug 28, 2023

I do agree it looks better with a tiny bit more vertical margin. I would also like to add some tiny bit of horizontal margin, but I'll do that in #26552.

@silverwind
Copy link
Member

It was some sort of issue if you combine flex-item and "ui attached segment"

Could it be resolved by .ui.attached.segment + .flex-list ?

Let's not create a dependeny on fomantic classes, if possible.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Aug 28, 2023

It was some sort of issue if you combine flex-item and "ui attached segment"

Could it be resolved by .ui.attached.segment + .flex-list ?

Let's not create a dependeny on fomantic classes, if possible.

The missing padding-top breaks other layout. You could take a try.

I wouldn't say the paddings in not(:last-child) and .flex-item + .flex-item are the right approach.

@wxiaoguang
Copy link
Contributor

It's better to keep children elements simple, and let parent containers layout the necessary padding/margin.

-> Improve flex list item padding #26779

@lunny lunny closed this Aug 29, 2023
@lunny lunny deleted the lunny/adjust_repo_list_padding_top_bottom branch August 29, 2023 02:59
silverwind pushed a commit that referenced this pull request Aug 29, 2023
Replace #26761

It's better to keep children elements simple, and let parent containers
layout the necessary padding/margin.

The old `not(:last-child)` and `.flex-item + .flex-item` are not easy to
maintain (for example, what if the developer would like to use a "tiny
height" item?)

The old approach also makes some UI look strange because the first item
doesn't have proper padding-top.

In this PR, we just simply use `.flex-item { padding: ... }`:

* Developers could manually set the item height they want easily
* It's easier to make it work with various containers -- with padding
(`ui segment`) and without padding (`div`)

And added more samples/examples.


![image](https://github.com/go-gitea/gitea/assets/2114189/719ea712-0241-4426-b67f-5723993c4ed7)

Co-authored-by: Giteabot <teabot@gitea.io>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants