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

Restructure issue list template, styles #25750

Merged
merged 7 commits into from
Jul 9, 2023

Conversation

denyskon
Copy link
Member

@denyskon denyskon commented Jul 7, 2023

This PR does various modifications on the issue list shared template:

  • restructure layout to achieve better responsiveness
  • fix various style issues
  • restructure styles (better result with less code :)
  • remove numerous gt-* patches and other unneeded classes -> use existing css classes
Before:

Bildschirmfoto vom 2023-07-07 14-35-00
Bildschirmfoto vom 2023-07-07 14-35-19
Bildschirmfoto vom 2023-07-07 14-35-43

After:

Bildschirmfoto vom 2023-07-07 14-32-04
Bildschirmfoto vom 2023-07-07 14-31-32
Bildschirmfoto vom 2023-07-07 14-31-14

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 7, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 7, 2023
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jul 7, 2023
@silverwind
Copy link
Member

One idea I had in mind is to move from .issue.list to a better name and a single class name. The styles originate from the issue list but a number of other lists now use it as well, so the name is not accurate any more. We'd need a somewhat generic name, maybe call it flex-list or similar. Doesn't have to be in this PR of course.

@denyskon
Copy link
Member Author

denyskon commented Jul 7, 2023

@silverwind I plan to merge all the similar & often partially broken 'card styles/templates' into one at some point as most of them use a similar layout: leading icon/image, title, subtitle/body, trailing icons/buttons. I thought it would be better to start with this small refactoring.

I would prefer if we could merge this now, and an/some additional PR(s) will follow soon continuing with this approach.

@silverwind
Copy link
Member

Yes, of course. Step by step.

@denyskon denyskon added type/bug topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Jul 7, 2023
@denyskon
Copy link
Member Author

denyskon commented Jul 8, 2023

I have started refactoring the other card elements to use this card structure, and it looks like it's easier to do it in one PR so nothing breaks in between...

Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

It looks clearer than before, and it fixes more SVG alignments.

@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 Jul 9, 2023
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 9, 2023
@GiteaBot
Copy link
Contributor

GiteaBot commented Jul 9, 2023

@denyskon please fix the merge conflicts. 🍵

@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 9, 2023
@wxiaoguang
Copy link
Contributor

The conflicts are caused by Reformat some templates #25756

That's what I said in #25739 (comment)

No reformat. To avoid unnecessary diff and unnecessary merge conflicts
...
It's not a weak reason now since there are a lot of frontend refactoring.
Otherwise some PRs would lose changes when resolving conflicts, for example, I fixed merge errors in #25625.....
If there is no "indent" change, git can merge most of the changes flawlessly.

@silverwind
Copy link
Member

silverwind commented Jul 9, 2023

Maybe if we could get the backend to just "minify" the insignificant HTML whitespace, it would be ideal, but I'm not sure whether the extra the processing cost of such an effort is worth it.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Jul 9, 2023

To help merge this PR, you can do this:

  1. Re-format the conflicted templates on your side following Reformat some templates #25756
  2. Then do a git merge, then git should be able to auto merge most changes without losing code

@silverwind
Copy link
Member

silverwind commented Jul 9, 2023

Could probably do template optimization as a one-time action on process startup and then cache the result in memory.

@wxiaoguang
Copy link
Contributor

I do not think it's worth to play with these spaces at the moment, neither the spaces between tags nor the spaces between class names.

Just like you won't look into the generated assemble code in a binary program, won't look into the generated JS code after packing, nobody would be hurt by the generated HTML code for browsers -- as long as the browser can render it correctly.

Code should be written in a developer-friendly style, but not in a generated-content-friendly style.

@silverwind
Copy link
Member

Agree it's not worth from a review standpoint, but delivering minimal HTML is worth from performance standpoint.

@denyskon
Copy link
Member Author

denyskon commented Jul 9, 2023

Fixed the conflicts

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 9, 2023
@silverwind silverwind enabled auto-merge (squash) July 9, 2023 18:39
@silverwind silverwind merged commit be23b73 into go-gitea:main Jul 9, 2023
@GiteaBot GiteaBot added this to the 1.21.0 milestone Jul 9, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Jul 9, 2023
@denyskon denyskon deleted the fix/issue-list branch July 9, 2023 22:47
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jul 10, 2023
* giteaofficial/main: (31 commits)
  Fix WORK_DIR for docker (root) image (go-gitea#25738)
  Avoid amending the Rebase and Fast-forward merge if there is no message template (go-gitea#25779)
  Show edit title button on commits tab of PR, too (go-gitea#25791)
  Make "install page" respect environment config (go-gitea#25648)
  Enable H014 and H023 djlint rules (go-gitea#25786)
  Restructure issue list template, styles (go-gitea#25750)
  Fix notification list bugs (go-gitea#25781)
  Revert package access change from go-gitea#23879 (go-gitea#25707)
  Make route middleware/handler mockable (go-gitea#25766)
  Update tool dependencies, lock govulncheck and actionlint (go-gitea#25655)
  Test if container blob is accessible before mounting (go-gitea#22759)
  Always pass 6-digit hex color to monaco (go-gitea#25780)
  Fix the wrong default branch name displayed by checkout (go-gitea#25777)
  Tweak repo topics bar (go-gitea#25769)
  docs: rootless docker ssh's default port is 2222 (go-gitea#25771)
  Repository Archived text title center align (go-gitea#25767)
  Update JS dependencies, misc tweaks (go-gitea#25768)
  Clarify "text-align" CSS helpers, fix clone button padding (go-gitea#25763)
  Switch to `vite-string-plugin` (go-gitea#25762)
  Newly pushed branches hints on repository home page (go-gitea#25715)
  ...
silverwind pushed a commit that referenced this pull request Jul 31, 2023
This PR introduces a new UI element type for Gitea called `flex-item`.
It consists of a horizontal card with a leading, main and trailing part:


![grafik](https://github.com/go-gitea/gitea/assets/47871822/395dd3f3-3906-4481-8f65-be6ac0acbe03)

The idea behind it is that in Gitea UI, we have many cases where we use
this kind of layout, but it is achieved in many different ways:
  - grid layout
  - `.ui.list` with additional hacky flexbox
- `.ui.key.list` - looks to me like a style set originally created for
ssh/gpg key list, was used in many other places
  - `.issue.list` - created for issue cards, used in many other places
  - ...

This new style is based on `.issue.list`, specifically the refactoring
of it done in #25750.

In this PR, the new element is introduced and lots of templates are
being refactored to use that style. This allows to remove a lot of
page-specific css, makes many of the elements responsive or simply
provides a cleaner/better-looking way to present information.

A devtest section with the new style is also available.

<details>
<summary>Screenshots (left: before, right: after)</summary>

![Bildschirmfoto vom 2023-07-09
21-01-21](https://github.com/go-gitea/gitea/assets/47871822/545b7da5-b300-475f-bd6d-b7d836950bb5)
![Bildschirmfoto vom 2023-07-09
21-01-56](https://github.com/go-gitea/gitea/assets/47871822/b6f70415-6795-4f71-a5ea-117d56107ea1)
![Bildschirmfoto vom 2023-07-09
21-02-45](https://github.com/go-gitea/gitea/assets/47871822/47407121-3f2a-4778-8f6d-ad2687c2e7b3)
![Bildschirmfoto vom 2023-07-09
21-03-44](https://github.com/go-gitea/gitea/assets/47871822/76167aaf-c3b2-46f6-9ffd-709f20aa6a34)
![Bildschirmfoto vom 2023-07-09
21-04-52](https://github.com/go-gitea/gitea/assets/47871822/af8fdde5-711e-4524-99cf-fb5d68af85b9)
![Bildschirmfoto vom 2023-07-09
21-05-25](https://github.com/go-gitea/gitea/assets/47871822/ae406946-e3e4-4109-abfe-b3588a07b468)
![Bildschirmfoto vom 2023-07-09
21-06-35](https://github.com/go-gitea/gitea/assets/47871822/2dbacc04-24d6-4f91-9e42-e16d6e4b5f1f)
![Bildschirmfoto vom 2023-07-09
21-09-03](https://github.com/go-gitea/gitea/assets/47871822/d3ca4e56-a72f-4179-adc8-98bfd638025b)
![Bildschirmfoto vom 2023-07-09
21-09-44](https://github.com/go-gitea/gitea/assets/47871822/df1fa689-499c-4e54-b6fb-3b81644b725f)
![Bildschirmfoto vom 2023-07-09
21-10-27](https://github.com/go-gitea/gitea/assets/47871822/b21cac71-a85a-4c8c-bb99-ab90373d8e09)
![Bildschirmfoto vom 2023-07-09
21-11-12](https://github.com/go-gitea/gitea/assets/47871822/89be39cf-0af9-4f2d-9fca-42f9eb5e7824)
![Bildschirmfoto vom 2023-07-09
21-12-01](https://github.com/go-gitea/gitea/assets/47871822/079579ea-1ecb-49c0-b32b-b59ed957caec)
![Bildschirmfoto vom 2023-07-09
21-17-44](https://github.com/go-gitea/gitea/assets/47871822/61ac6ec4-a319-4d5c-9c99-2e02a77295ba)
![Bildschirmfoto vom 2023-07-09
21-18-27](https://github.com/go-gitea/gitea/assets/47871822/5b55b73f-6244-47f7-a3e6-c5e4a7474585)
![Bildschirmfoto vom 2023-07-09
21-19-18](https://github.com/go-gitea/gitea/assets/47871822/c1b7c22e-3e5a-46d4-b8d6-5560db478c0b)
![Bildschirmfoto vom 2023-07-09
21-29-13](https://github.com/go-gitea/gitea/assets/47871822/82ffca8d-ab2e-4a18-9954-5b685bf6a422)
![Bildschirmfoto vom 2023-07-09
21-30-11](https://github.com/go-gitea/gitea/assets/47871822/ad2fdccc-2be8-41bb-bfdc-a084aa387b61)
![Bildschirmfoto vom 2023-07-09
21-32-44](https://github.com/go-gitea/gitea/assets/47871822/2d298ba7-d084-48b5-a139-f86d56262110)
![Bildschirmfoto vom 2023-07-09
21-33-28](https://github.com/go-gitea/gitea/assets/47871822/4cbd838e-9de8-4ad0-8ed9-438da5c9a5cb)


</details>

---------

Co-authored-by: Giteabot <teabot@gitea.io>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Oct 7, 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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/ui Change the appearance of the Gitea UI type/bug type/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants