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 first section on home page full width #23854

Merged
merged 2 commits into from
Apr 1, 2023
Merged

Make first section on home page full width #23854

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c39d721
Make first section on home page full width
silverwind Mar 31, 2023
a6816c8
Merge branch 'main' into fullwidth-home
techknowlogick Apr 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion templates/home.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{{template "base/head" .}}
<div role="main" aria-label="{{if .IsSigned}}{{.locale.Tr "dashboard"}}{{else}}{{.locale.Tr "home"}}{{end}}" class="page-content home">
<div class="ui stackable middle very relaxed page grid">
<div class="ui middle very relaxed page gt-mb-5">
Copy link
Contributor

@wxiaoguang wxiaoguang Apr 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It's not a grid anymore, does the code sixteen wide column below make sense? Or should they be cleaned up?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, I think those classes are dead now. But still, I left them in because this seems like a common target for customization, and I don't want to unnecessarily break those.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since you have removed the grid on its parent, keeping the dead code doesn't help about avoiding "breaking", instead it might really cause breaks if the customization depends on the grid.

And, I worry about that the dead code might cause other style conflicts in the future or unnecessary technical debt / copy-paste bug in the future.

So I guess we should remove dead code as soon as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess we could also just add gt-px-0 to the classes and keep the old ones. Same effect and even more compatible.

Copy link
Contributor

Choose a reason for hiding this comment

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

TBH I do not think so ....

If a developer reads code:

<div class="ui ...."> 
   <div class="six wide column">
   </div>
</div>

They will think about: OK , the inner div is designed to take 6/12 or 6/24 of the screen.

But the truth is the "six wide column" is a no-op now. It really surprises developers, I do not like such surprise.

There are already a lot of surprises in Gitea's codebase. To improve maintainability and readability, let's try to avoid surprises?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe you could file a new PR with your envisioned changes. I don't quite understand what is requested anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem. Improve home page template #23856

Major changes:

  1. The old <div class="ui"><div class="six wide column ..."> </div></div> doesn't have affect any more
    • So clean them, and remove other unnecessary elements/styles.
  2. Add padding for narrow view.

<div class="sixteen wide center aligned centered column">
<div>
<img class="logo" width="220" height="220" src="{{AssetUrlPrefix}}/img/logo.svg" alt="{{.locale.Tr "logo"}}">
Expand Down