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

Improve "language stats" UI #26968

Merged
merged 13 commits into from
Sep 10, 2023
Merged

Improve "language stats" UI #26968

merged 13 commits into from
Sep 10, 2023

Conversation

wxiaoguang
Copy link
Contributor

Before:

After:

  • Simplify the code
  • The UI doesn't flicker

image

image

@wxiaoguang wxiaoguang added topic/ui Change the appearance of the Gitea UI type/refactoring Existing code has been cleaned up. There should be no new functionality. labels Sep 8, 2023
@wxiaoguang wxiaoguang added this to the 1.21.0 milestone Sep 8, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 8, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Sep 8, 2023
@silverwind
Copy link
Member

If by "flicker" you mean the slight layout shift during animation, that would be great to fix, it's been bothering me a long time.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Sep 8, 2023 •
edited
Loading

If by "flicker" you mean the slight layout shift during animation ...

Yes, the layout shifted slightly (ps: there is no animation after the removal of jQuery slideToggle), it has been fixed.

@silverwind
Copy link
Member

Yeah, don't need that animation either.

@silverwind
Copy link
Member

silverwind commented Sep 8, 2023 •
edited
Loading

Pushed a fix for border-radius of the language bar:

image

overflow: hidden is necessary for it to work. Tooltips still work because they are portalled. Fomantic's default radius almost worked, but I had to fine-tune it a bit to look perfect.

@silverwind
Copy link
Member

Reduced gap a bit to help the menu on mobile where it is starved for space:

Screenshot 2023-09-08 at 14 52 11

@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 Sep 8, 2023
@wxiaoguang
Copy link
Contributor Author

Reduced gap a bit to help the menu on mobile where it is starved for space:

I will add some extra padding and overflow: hidden. Because some language texts might be pretty long

@silverwind
Copy link
Member

silverwind commented Sep 8, 2023 •
edited
Loading

For this 4-item menu, I ideally would like to lay it out in 2 rows with 2 items each when space is not enough, but flex-wrap can not do such things as it would wrap only a single item onto second row, it would probably need display: grid.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Sep 8, 2023 •
edited
Loading

The overflow is unavoidable, so I think it's fine to just let it overflow and get hidden

image

image

@silverwind
Copy link
Member

Now it cuts off the size, but I guess this is better than before:

image

Those numbers can easily go 4-digits each and translation is also variable width. I think ideally we need 2-row layout.

@silverwind
Copy link
Member

Maybe just hide size at mobile breakpoint.

@silverwind
Copy link
Member

I'll push that momentarily.

@wxiaoguang
Copy link
Contributor Author

Maybe just hide size at mobile breakpoint.

Yup, the size could be hidden. While the "overflow: hidden" is still needed, because the language list would have many items (unpredictable).

@github-actions github-actions bot added the type/docs This PR mainly updates/creates documentation label Sep 8, 2023
@silverwind
Copy link
Member

Better now with it hidden:

Screenshot 2023-09-08 at 15 11 03

@github-actions github-actions bot removed the type/docs This PR mainly updates/creates documentation label Sep 8, 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 Sep 10, 2023
@denyskon denyskon added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 10, 2023
@wxiaoguang wxiaoguang merged commit dd6e8ab into go-gitea:main Sep 10, 2023
26 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 10, 2023
@wxiaoguang wxiaoguang deleted the fix-language-ui branch September 10, 2023 10:27
zjjhot added a commit to zjjhot/gitea that referenced this pull request Sep 11, 2023
@zjjhot
Merge remote-tracking branch 'upstream/main'
793a96b 
* upstream/main:
  [skip ci] Updated translations via Crowdin
  Add some more labels to labeler (go-gitea#26987)
  Fix INI parsing for value with trailing slash (go-gitea#26995)
  Correct the database.LOG_SQL default value in config cheat sheet (go-gitea#26997)
  Improve "language stats" UI (go-gitea#26968)
  [skip ci] Updated translations via Crowdin
  Update chroma to v2.9.1 (go-gitea#26990)
  Improve issue list layout (go-gitea#26983)
@wxiaoguang wxiaoguang mentioned this pull request Oct 1, 2023
Closed
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind approved these changes

@denyskon denyskon denyskon approved these changes

Assignees
No one assigned
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/refactoring Existing code has been cleaned up. There should be no new functionality.
Projects
None yet
Milestone
1.21.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@wxiaoguang @silverwind @denyskon @GiteaBot @lunny