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

Highlight focused diff file #22870

Merged
merged 3 commits into from
Feb 13, 2023
Merged

Highlight focused diff file #22870

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ec67fd
Highlight selected diff file
delvh Feb 11, 2023
df63341
Appease linter
delvh Feb 11, 2023
74aa349
Merge branch 'main' into feature/highlight-target-diff
lunny Feb 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions web_src/less/_review.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,11 @@ a.blob-excerpt:hover {
width: 72px;
height: 10px;
}

.diff-file-box {
border-radius: .285rem; // Just like ui.top.attached.header
}

.diff-file-box:target {
box-shadow: 0 0 0 3px var(--color-accent);
}
Comment on lines +320 to +327
Copy link
Contributor

@wxiaoguang wxiaoguang Feb 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a .diff-file-box in

gitea/web_src/less/_repository.less

Line 1706 in be93e48

.diff-file-box {

And the border-radius doesn't look necessary, or it should be fine tuned on .diff-file-box:target. On your screenshot, the bottom border corners are still right angles.

Screenshot:

image

Copy link
Member Author

@delvh delvh Feb 12, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. _repository.less is a completely different file, and the selector you're speaking about is nested. While we can use it, I'm not a fan of nested selectors as they make the outcome so difficult to predict. Additionally, _review should be loaded less often than _repository and only when needed.
  2. The top border-radius is needed as the diff-file-header sets the border-radius: 0.285rem (ui.top.attached.header). Otherwise, it looks like this:
    no border-radius. What we could think about is moving the border-radius into :target, but why?
    Why should it change its boundaries only when targeted?
  3. I know that as well. However, I've wasted half an hour already on it and still haven't found any solution. The problem seems to be that Fomantic UI forcefully sets the border-radius:0 for the table, and I haven't found any solution yet. At that point, I simply surrendered and I think the functional aspect of this enhancement is present. If anyone has any better ideas, please feel free to commit directly on this PR.
  4. (GitHub themselves haven't fixed this either :) )