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

File picker broken #23593

Closed
gempir opened this issue Mar 20, 2023 · 10 comments · Fixed by #23553
Closed

File picker broken #23593

gempir opened this issue Mar 20, 2023 · 10 comments · Fixed by #23553
Labels
type/bug
Milestone
1.19.1

Comments

@gempir
Copy link
Contributor

gempir commented Mar 20, 2023 •
edited
Loading

Description

The file picker in a PR has a tiny height and is effectively useless. Tested on 1.19.0 was not broken on 1.19.0-rc1

Example test PR
https://try.gitea.io/gempir/gempbot/pulls/1/files

Gitea Version

1.19.0

Can you reproduce the bug on the Gitea demo site?

Yes

Log Gist

No response

Screenshots

image

image

Git Version

No response

Operating System

No response

How are you running Gitea?

systemd

Database

None
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 20, 2023 •
edited
Loading

It (update:expanding) works, but ....... there is a height (update:scrolling) bug 😭

I guess it's a regression of the LESS->CSS, but I can't tell the difference at the moment ..... according to #23593 (comment) , maybe there is other problem.

Add a screenshot with details for future readers.

image

@lunny lunny added this to the 1.19.1 milestone Mar 20, 2023
@silverwind
Copy link
Member

silverwind commented Mar 20, 2023 •
edited
Loading

#23553 includes a fix for it (removing height: 100%, see #23553 (comment)). I don't think it's from the CSS conversion, just an edge case with many files that was not found earlier.

@gempir
Copy link
Contributor Author

gempir commented Mar 20, 2023

I did add screenshots, maybe they were not clear enough?

I don't really see how a PR with a scrollbar in the File Picker is an edge case to me that's a regular occurence, as we noticed this regression like 2 hours after upgrading.

@silverwind
Copy link
Member

"Edge case" is probably not the best term, but I mean the bug does not show in all cases.

@wxiaoguang
Copy link
Contributor

I did add screenshots, maybe they were not clear enough?

Sorry for bothering, at first I just thought that "expanding" doesn't work well with some height, so I confirmed again that it seems only related to the height & scrolling ..... (correct me if I am wrong)

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Mar 20, 2023 •
edited
Loading

I guess it's still somewhat related to the LESS->CSS (the change itself is good and right, but maybe some old bugs are triggered?) nope, it's #23554

According to my test, before: 202803f , the tree-list just works (I can't reproduce the height bug), with current main , I can 100% reproduce it .... strange.

I just don't understand what causes the problem ..... I will spend some time on it to see if we can find the root problem, to avoid other potential bugs.

Update: the related PR is #23554 , maybe it's not a edge case, but a regression?

@silverwind silverwind mentioned this issue Mar 20, 2023
Merged
@silverwind
Copy link
Member

Yes, it is a regression from #23554 and #23553 will fix it.

@silverwind silverwind mentioned this issue Mar 21, 2023
Merged
@jolheiser jolheiser mentioned this issue Mar 22, 2023
Closed
jolheiser pushed a commit that referenced this issue Mar 22, 2023
@silverwind
Fix diff tree height and adjust target file style (#23616)
622d216 
Extract from #23553, just the
parts that fix the diff tree height and the change to the file `:target`
style.

Fixes: #23593
@wxiaoguang
Copy link
Contributor

The fix for 1.19 (#23616) has been merged.

You can get 1.19 latest from https://dl.gitea.com/gitea/1.19/ , or docker gitea:1.19-dev

These contain the fix.

@gempir
Copy link
Contributor Author

gempir commented Mar 23, 2023

Will there be no 1.19.1 release ?

@wxiaoguang
Copy link
Contributor

There will be. Using 1.19-dev is the most convenient way to get bug fixes quickly before 1.19.1 gets released.

All fixed in 1.19-dev will be included in 1.19.1

After 1.19.1 gets release, then 1.19-dev is for next 1.19.2

@wxiaoguang wxiaoguang mentioned this issue Mar 23, 2023
Closed
lunny pushed a commit that referenced this issue Mar 30, 2023
@silverwind
Diff improvements (#23553)
aa4d1d9 
- Avoid flash of wrong tree toggle icon on page load by setting icon
based on sync state
- Avoid "pop-in" of tree on page load by leaving space based on sync
state
- Use the same border/box-shadow combo used on comment `:target` also
for file `:target`.
- Refactor `DiffFileTree.vue` to use `toggleElem` instead of hardcoded
class name.
- Left-align inline comment boxes and make them fit the same amount of
markup content on a line as GitHub.
- Fix height of `diff-file-list`

Fixes: #23593

<img width="1250" alt="Screenshot 2023-03-18 at 00 52 04"
src="https://user-images.githubusercontent.com/115237/226071392-6789a644-aead-4756-a77e-aba3642150a0.png">
<img width="1246" alt="Screenshot 2023-03-18 at 00 59 43"
src="https://user-images.githubusercontent.com/115237/226071443-8bcba924-458b-48bd-b2f0-0de59cb180ac.png">
<img width="1250" alt="Screenshot 2023-03-18 at 01 27 14"
src="https://user-images.githubusercontent.com/115237/226073121-ccb99f9a-d3ac-40b7-9589-43580c4a01c9.png">
<img width="1231" alt="Screenshot 2023-03-19 at 21 44 16"
src="https://user-images.githubusercontent.com/115237/226207951-81bcae1b-6b41-4e39-83a7-0f37951df6be.png">

(Yes I'm aware the border-radius in bottom corners is suboptimal, but
this would be notorously hard to fix without relying on `overflow:
hidden`).
@wxiaoguang wxiaoguang mentioned this issue Apr 2, 2023
Closed
@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.
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
1.19.1
Development

Successfully merging a pull request may close this issue.

Diff improvements silverwind/gitea
5 participants
@lunny @silverwind @techknowlogick @wxiaoguang @gempir