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

Make first section on home page full width #23854

Merged
merged 2 commits into from
Apr 1, 2023

Conversation

silverwind
Copy link
Member

Before:
Screenshot 2023-03-31 at 19 56 16

After:
Screenshot 2023-03-31 at 20 00 14

@silverwind silverwind added the type/enhancement An improvement of existing functionality label Mar 31, 2023
@silverwind silverwind added this to the 1.20.0 milestone Mar 31, 2023
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 31, 2023
@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label Mar 31, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 1, 2023
@techknowlogick techknowlogick enabled auto-merge (squash) April 1, 2023 00:34
@techknowlogick
Copy link
Member

🤖 🎺

@techknowlogick techknowlogick merged commit 053df15 into go-gitea:main Apr 1, 2023
@@ -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

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.

lunny pushed a commit that referenced this pull request Apr 1, 2023
Follow #23854

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.

Before

![image](https://user-images.githubusercontent.com/2114189/229262177-e8cf6c9b-b17b-482c-83fe-a84579e01e8e.png)

After:

![image](https://user-images.githubusercontent.com/2114189/229262166-d46134b0-2117-4d5c-a469-a2115cbd4b6c.png)
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 3, 2023
* upstream/main:
  [skip ci] Updated translations via Crowdin
  Update JS deps (go-gitea#23853)
  Added close/open button to details page of milestone (go-gitea#23877)
  Check `IsActionsToken` for LFS authentication (go-gitea#23841)
  Prefill input values in oauth settings as intended (go-gitea#23829)
  Display image size for multiarch container images (go-gitea#23821)
  Use clippie module to copy to clipboard (go-gitea#23801)
  Remove assertion debug code for show/hide refactoring (go-gitea#23576)
  [skip ci] Updated translations via Crowdin
  Remove jQuery ready usage (go-gitea#23858)
  Fix JS error when changing PR's target branch (go-gitea#23862)
  Improve action log display with control chars (go-gitea#23820)
  Fix review conversation reply (go-gitea#23846)
  Improve home page template, fix Sort dropdown menu flash (go-gitea#23856)
  Make first section on home page full width (go-gitea#23854)
  [skip ci] Updated translations via Crowdin
  Fix incorrect CORS failure detection logic (go-gitea#23844)
@go-gitea go-gitea locked and limited conversation to collaborators May 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants