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

Fix the overflow style for "Hide all checks" #27932

Merged
merged 11 commits into from
Nov 7, 2023
Merged

Fix the overflow style for "Hide all checks" #27932

merged 11 commits into from
Nov 7, 2023

Conversation

wxiaoguang
Copy link
Contributor

Fix #27928

@wxiaoguang wxiaoguang added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Nov 6, 2023
@wxiaoguang wxiaoguang added this to the 1.22.0 milestone Nov 6, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 6, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 6, 2023
@wxiaoguang wxiaoguang mentioned this pull request Nov 6, 2023
Closed
@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 Nov 6, 2023
silverwind
silverwind reviewed Nov 6, 2023
View reviewed changes
web_src/css/repo.css Outdated Show resolved Hide resolved
@silverwind
Copy link
Member

silverwind commented Nov 6, 2023 •
edited

Did some more fixes surrounding the borders. Now the header has the border and there's one between each item. It looks good with MacOS elastic overscroll now.

I have to say I find this JS less readable than before with the data-toggled property, but I can accept it.

image

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

wxiaoguang commented Nov 7, 2023 •
edited

I have to say I find this JS less readable than before with the data-toggled property, but I can accept it.

We could hugely simplify it to something like data-toggled, but the annoying scrollbar would appear.

border: none !important;

I think the old code is the right thing to do instead of border-top/border-right/border-left.

Each item had its own border-top, everything should be OK, there is no need to tweak header.style.borderBottom, and the height could be 39*4=234.

I do not understand the new change.

@wxiaoguang wxiaoguang marked this pull request as draft November 7, 2023 00:24
@wxiaoguang wxiaoguang added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Nov 7, 2023
@silverwind
Copy link
Member

I do not understand the new change.

On Mac, scroll down a bit then scroll up fast with elastic scrolling enabled. Previously it showed the border-top on the first item which was ugly. Now that the border is on the header and between items, it looks better.

@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 7, 2023
@pull-request-size pull-request-size bot removed the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 7, 2023
@pull-request-size pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 7, 2023
@wxiaoguang
Copy link
Contributor Author

I do not understand the new change.

On Mac, scroll down a bit then scroll up fast with elastic scrolling enabled. Previously it showed the border-top on the first item which was ugly. Now that the border is on the header and between items, it looks better.

Do you mean this? a03d597

image

image

@wxiaoguang wxiaoguang removed the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Nov 7, 2023
@wxiaoguang wxiaoguang marked this pull request as ready for review November 7, 2023 06:30
@silverwind silverwind self-requested a review November 7, 2023 08:20
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. labels Nov 7, 2023
@silverwind
Copy link
Member

silverwind commented Nov 7, 2023 •
edited

Border-top issue has returned with latest change. Will fix:

top

@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 Nov 7, 2023
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 7, 2023
@silverwind silverwind enabled auto-merge (squash) November 7, 2023 18:43
@silverwind silverwind merged commit 10a6ebb into go-gitea:main Nov 7, 2023
25 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Nov 7, 2023
@wxiaoguang wxiaoguang deleted the fix-hide-checks branch November 8, 2023 03:08
zjjhot added a commit to zjjhot/gitea that referenced this pull request Nov 8, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
cf839bf 
* giteaofficial/main:
  Fix citation error when the file size is larger than 1024 bytes (go-gitea#27958)
  Use flex-container on user dashboard (go-gitea#27956)
  Document REACTION_MAX_USER_NUM setting option (go-gitea#27954)
  Add word-break to repo description in home page (go-gitea#27924)
  Remove go versions from .golangci.yml (go-gitea#27953)
  Fix the overflow style for "Hide all checks" (go-gitea#27932)
  Fix rendering assignee changed comments without assignee (go-gitea#27927)
  Add word break to release title (go-gitea#27942)
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
@wxiaoguang @silverwind @fuxiaohei
Fix the overflow style for "Hide all checks" (go-gitea#27932)
171260f 
Fix go-gitea#27928

---------

Co-authored-by: silverwind <me@silverwind.io>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Feb 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@lunny lunny lunny approved these changes

@silverwind silverwind silverwind 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/S Denotes a PR that changes 10-29 lines, ignoring generated files. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features.
Projects
None yet
Milestone
1.22.0
Development

Successfully merging this pull request may close these issues.

After hiding and unhiding commit checks, there are only four checks available
4 participants
@wxiaoguang @silverwind @lunny @GiteaBot