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

Repo file list enhancements #32835

Merged
merged 14 commits into from
Dec 15, 2024
Merged

Repo file list enhancements #32835

merged 14 commits into from
Dec 15, 2024

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Dec 14, 2024

  1. restore background color
  2. fix border radius on top/bottom and on hover
  3. changed row border from 1px bottom to .5px top and bottom so it looks equal on both sides when hovering. i think this is a problem introduced by display: contents, not sure if there is a better solution.
  4. parent link is now full-row again, much easier to click
  5. parent link now uses directory icon, matching github
  6. changed grid layout to remove auto width on file name column which could get too small.
  7. mobile layout now shows more of the filename.
image Screenshot 2024-12-14 at 04 29 28 Screenshot 2024-12-14 at 04 40 32

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 14, 2024
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 14, 2024
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Dec 14, 2024
@silverwind
Copy link
Member Author

BTW I noticed one regression which I'm not sure yet how to solve. The ... button is invisible:

image

@wxiaoguang
Copy link
Contributor

4. changed row border from 1px bottom to .5px top and bottom so it looks equal on both sides when hovering. i think this is a problem introduced by display: contents, not sure if there is a better solution.

I do not see why. The layout is:

--- boder-top ----
padding-top
text
padding-bottom
--- boder-top ---- (of next item)

Why it doesn't look equal?

@wxiaoguang
Copy link
Contributor

BTW I noticed one regression which I'm not sure yet how to solve. The ... button is invisible:

It doesn't seem to be a regression.

@silverwind
Copy link
Member Author

silverwind commented Dec 14, 2024

  1. changed row border from 1px bottom to .5px top and bottom so it looks equal on both sides when hovering. i think this is a problem introduced by display: contents, not sure if there is a better solution.

I do not see why. The layout is:

--- boder-top ----
padding-top
text
padding-bottom
--- boder-top ---- (of next item)

Why it doesn't look equal?

Here is how it looked before:

image

border-top of item below not visible here because background overlaps it. I think this is because of display: contents.

@silverwind
Copy link
Member Author

silverwind commented Dec 14, 2024

BTW I noticed one regression which I'm not sure yet how to solve. The ... button is invisible:

It doesn't seem to be a regression.

We can handle it in another PR. It will require changes to .commit-summary which is tricky to modify because the class is also used in commit list and IIRC single commit view.

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 14, 2024

Here is how it looked before:
border-top of item below not visible here because background overlaps it. I think this is because of display: contents.

Strange, the border-top of item below is actually there on my side:

image

@silverwind
Copy link
Member Author

Seems this is a Firefox bug, but the display on Chrome is not perfect either because border colors do not match on hover:

image

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 14, 2024

Seems this is a Firefox bug, but the display on Chrome is not perfect either because border colors do not match on hover:

Unable to produce on my Firefox 133.0.3 (aarch64)

image

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Dec 14, 2024

And the Chrome color seems right too

image

@silverwind
Copy link
Member Author

Hmm odd. On main branch, rendering does look same as Chrome in my Firefox.

image

So it must been something in my PR.

@silverwind
Copy link
Member Author

And the Chrome color seems right too

Check with a color picker, the borders are not equal color.

@wxiaoguang
Copy link
Contributor

And the Chrome color seems right too

Check with a color picker, the borders are not equal color.

It is caused by the alpha chanel: --color-hover: #00001708;

@silverwind
Copy link
Member Author

Yeah I think we need an opaque hover color, maybe there is one already as I recall having this problem in the past already.

@wxiaoguang
Copy link
Contributor

Yeah I think we need an opaque hover color, maybe there is one already as I recall having this problem in the past already.

So the 0.5px border still has the same problem 🤣

If it is not a must, I'd like to keep using the single "border-top: 1px", it should be clearer.

* origin/main:
  Refactor markdown math render (go-gitea#32831)
  Add User-Agent for gitea's self-implemented lfs client. (go-gitea#32832)
@silverwind
Copy link
Member Author

Yeah I will likely revert to 1px and make the color opaque. It does seem I need to add a new CSS variable unfortunately.

@silverwind silverwind marked this pull request as draft December 14, 2024 05:54
@silverwind
Copy link
Member Author

WIP until I fix the hover border color issue.

@wxiaoguang
Copy link
Contributor

Actually there could be another choice without using the "border trick"

<div divider>
<div item> name message age</div>
<div divider>
<div item> name message age</div>

Make the "divider" full line in the grid

@silverwind
Copy link
Member Author

Note sure how I feel about introducing divider elements, it seems clumsy.

@silverwind
Copy link
Member Author

Fixed the border issue by adding opaque color:

image

Ideally I would like to use color-mix, but this would break PaleMoon (https://repo.palemoon.org/MoonchildProductions/UXP/issues/2489), so I hardcoded the color until then.

@silverwind silverwind marked this pull request as ready for review December 15, 2024 02:37
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.

much clearer than before

@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 Dec 15, 2024
@wxiaoguang wxiaoguang added this to the 1.23.0 milestone Dec 15, 2024
@silverwind
Copy link
Member Author

There is another minor issue with inequal top/bottom border color because it is not opaque either, I will change to a opaque one.

@silverwind
Copy link
Member Author

Borders completely fixed now. Internal border is one shade darker because I think it looks better.

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 Dec 15, 2024
@wxiaoguang wxiaoguang enabled auto-merge (squash) December 15, 2024 16:33
@wxiaoguang wxiaoguang merged commit 74b06d4 into go-gitea:main Dec 15, 2024
26 checks passed
@silverwind silverwind deleted the repotable branch December 16, 2024 03:02
@lunny lunny added the type/enhancement An improvement of existing functionality label Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/templates This PR modifies the template files size/M Denotes a PR that changes 30-99 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants