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

Login UI layout issue, the top marin of the login div missed. #29981

Closed
lunny opened this issue Mar 22, 2024 · 9 comments · Fixed by #29982
Closed

Login UI layout issue, the top marin of the login div missed. #29981

lunny opened this issue Mar 22, 2024 · 9 comments · Fixed by #29982
Labels
topic/ui Change the appearance of the Gitea UI type/bug

Comments

@lunny
Copy link
Member

lunny commented Mar 22, 2024

image
@lunny lunny added type/bug topic/ui Change the appearance of the Gitea UI labels Mar 22, 2024
@wxiaoguang
Copy link
Contributor

Regression of #29922? @silverwind

The margin of page-content was removed:

.page-content {
  margin-top: 15px;
}

@silverwind
Copy link
Member

silverwind commented Mar 22, 2024

Ah I did not see that because my login ui is different with the tab bar:

image

@silverwind
Copy link
Member

silverwind commented Mar 22, 2024

What exactly is necessary to reproduce the tabbar-less login ui?

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 22, 2024

I think .page-content really needs this margin-top

We should use margin for outer spaces as much as possible. They could overlap gracefully.

@silverwind
Copy link
Member

silverwind commented Mar 22, 2024

No, I think the content should itself decide whether it wants margin or not. Many tabbars want 0 margin. It's not for the .page-content to decide.

@silverwind
Copy link
Member

And @lunny don't call it broken. A minor layout issue is far from "broken" :)

@silverwind
Copy link
Member

The problem is the negative margins from fomantic on .ui.grid.

@lunny
Copy link
Member Author

lunny commented Mar 22, 2024

And @lunny don't call it broken. A minor layout issue is far from "broken" :)

Please help me to change it to a proper word.

@silverwind
Copy link
Member

silverwind commented Mar 22, 2024

I would call it "misaligned", or "incorrect render".

@lunny lunny changed the title Login UI broken, the top marin of the login div missed. Login UI layout issue, the top marin of the login div missed. Mar 22, 2024
silverwind added a commit to silverwind/gitea that referenced this issue Mar 22, 2024
silverwind added a commit that referenced this issue Mar 22, 2024
…ally (#29982)

Fixes: #29981. Introduce
`.secondary-nav` as a universal way for styling and margin adjustments
inside `.page-content`.

If the first child of `.page-content` is `.secondary-nav`, we add margin
below it, otherwise we add padding to the first child. Notable changes:

- `--color-header-wrapper` is replaced with `--color-secondary-nav-bg`.
- `navbar` class is removed.

---------

Co-authored-by: Giteabot <teabot@gitea.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
DennisRasey pushed a commit to DennisRasey/forgejo that referenced this issue Mar 28, 2024
…nt` spacing universally

Fixes: go-gitea/gitea#29981. Introduce
`.secondary-nav` as a universal way for styling and margin adjustments
inside `.page-content`.

If the first child of `.page-content` is `.secondary-nav`, we add margin
below it, otherwise we add padding to the first child. Notable changes:

- `--color-header-wrapper` is replaced with `--color-secondary-nav-bg`.
- `navbar` class is removed.

Co-authored-by: Giteabot <teabot@gitea.io>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

---

Conflict resolution: Trivial conflict & changed selector to reflect new
classes.
Ref: https://codeberg.org/forgejo/forgejo/issues/2776
(cherry picked from commit 3ccda41a539b8ba7841919ee12dc2877ddc03818)
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jun 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants