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 sticky header in diff view #23549

Closed
wants to merge 4 commits into from
Closed

Fix sticky header in diff view #23549

wants to merge 4 commits into from

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 17, 2023 •
edited
Loading

Fix regression #23513 (comment) from #23271. The previous sticky CSS did assume the content is always 2 rows, but since that PR, it's single-row above 993px width. Adjust the sticky offset to match and add a small tweak that hides content behind the border-radius.

Single row:
Screenshot 2023-03-17 at 21 33 05

Double row:
Screenshot 2023-03-17 at 21 32 53

@silverwind silverwind added type/bug topic/ui Change the appearance of the Gitea UI labels Mar 17, 2023
@silverwind silverwind added this to the 1.20.0 milestone Mar 17, 2023
@delvh delvh added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Mar 17, 2023
@silverwind silverwind mentioned this pull request Mar 17, 2023
Merged
@gempir
Copy link
Contributor

gempir commented Mar 17, 2023

This might be nit picking but I think the distance of 46.5px is incorrect.

Current 46.5px
image

The rounded corners on the top are cutoff.

50px looks more correct
image

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Mar 17, 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 Mar 17, 2023
@silverwind
Copy link
Member Author

silverwind commented Mar 17, 2023 •
edited
Loading

This 46.5px is fixing it for me, but I guess actual height depends on more factors like font height. We can try set a fixed height maybe.

@gempir
Copy link
Contributor

gempir commented Mar 17, 2023 •
edited
Loading

Likely not related but on some files I can scroll over them like this Dockerfile

image

and then the sticky becomes ever more wrong because now it only shows 2/3 of the header? Not sure what it should show at this point. This might be an old issue

image

@silverwind
Copy link
Member Author

Odd issue, I have no idea. A static value like 46.5px can't suddenly change like that. Check in devtools if you find out more.

@silverwind
Copy link
Member Author

silverwind commented Mar 17, 2023 •
edited
Loading

@gempir check again. Now the top row is fixed height and the second row offset matches it, so there is no possibilty for such mismatches anymore.

@gempir
Copy link
Contributor

gempir commented Mar 17, 2023

Yep that looks better on my device now, besides the unrelated issue mentioned above. But that's for another day. 👍

@silverwind
Copy link
Member Author

Moving these changes here to #23553 as I found more stuff to fix and these two PRs can't really be separated because of interactions.

@silverwind silverwind closed this Mar 18, 2023
@silverwind silverwind deleted the fix-sticky branch March 18, 2023 00:18
This was referenced Mar 18, 2023
Merged
Merged
@delvh delvh removed the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Mar 18, 2023
delvh pushed a commit that referenced this pull request Mar 18, 2023
@silverwind
Fix sticky header in diff view (#23554)
9efcce5 
Ressurection of #23549.

Fix regression #23513 (comment) from #23271.
The previous sticky CSS did assume the content is always 2 rows, but since that PR, it's single-row above 993px width.
Adjust the sticky offset to match and add a small tweak that hides content behind the `border-radius`.

Single row:
<img width="1264" alt="Screenshot 2023-03-17 at 21 33 05"
src="https://user-images.githubusercontent.com/115237/226034050-a04b131d-fd3f-45c0-bc72-413738a59825.png">

Double row:
<img width="1243" alt="Screenshot 2023-03-17 at 21 32 53"
src="https://user-images.githubusercontent.com/115237/226034163-2f1c6aa9-fc72-432f-bc46-9a7119da8677.png">
@GiteaBot GiteaBot mentioned this pull request Mar 18, 2023
Merged
GiteaBot pushed a commit to GiteaBot/gitea that referenced this pull request Mar 18, 2023
@silverwind @GiteaBot
Fix sticky header in diff view (go-gitea#23554)
dd7404e 
Ressurection of go-gitea#23549.

Fix regression go-gitea#23513 (comment) from go-gitea#23271.
The previous sticky CSS did assume the content is always 2 rows, but since that PR, it's single-row above 993px width.
Adjust the sticky offset to match and add a small tweak that hides content behind the `border-radius`.

Single row:
<img width="1264" alt="Screenshot 2023-03-17 at 21 33 05"
src="https://user-images.githubusercontent.com/115237/226034050-a04b131d-fd3f-45c0-bc72-413738a59825.png">

Double row:
<img width="1243" alt="Screenshot 2023-03-17 at 21 32 53"
src="https://user-images.githubusercontent.com/115237/226034163-2f1c6aa9-fc72-432f-bc46-9a7119da8677.png">
delvh pushed a commit that referenced this pull request Mar 19, 2023
@GiteaBot @silverwind
Fix sticky header in diff view (#23554) (#23568)
f5a98b0 
Backport #23554 by @silverwind

Ressurection of #23549.

Fix regression
#23513 (comment)
from #23271.
The previous sticky CSS did assume the content is always 2 rows, but
since that PR, it's single-row above 993px width.
Adjust the sticky offset to match and add a small tweak that hides
content behind the `border-radius`.

Single row:
<img width="1264" alt="Screenshot 2023-03-17 at 21 33 05"
src="https://user-images.githubusercontent.com/115237/226034050-a04b131d-fd3f-45c0-bc72-413738a59825.png">

Double row:
<img width="1243" alt="Screenshot 2023-03-17 at 21 32 53"
src="https://user-images.githubusercontent.com/115237/226034163-2f1c6aa9-fc72-432f-bc46-9a7119da8677.png">

Co-authored-by: silverwind <[email protected]>
@GiteaBot GiteaBot removed this from the 1.20.0 milestone Apr 23, 2023
@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.
Reviewers

@jolheiser jolheiser jolheiser approved these changes

Assignees
No one assigned
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@silverwind @gempir @jolheiser @GiteaBot @delvh