Skip to content

Conversation

zeripath
Copy link
Contributor

@zeripath zeripath commented Sep 8, 2020

This PR adds multiple improvements to GitGraphs:

  • PR heads can be excluded from view
  • Branch references within the commit list are now rendered as PRs, branches, tags or remotes and are clickable and will go to the branch/tag or PR
  • Branches/PRs/Tags can be chosen to be focused on
  • Commit signature verification and trust status is shown as are user avatars.

Also adds backend support for graphs on certain files.

Fix #10327

Signed-off-by: Andrew Thornton [email protected]

Screenshots

Basic View

Screenshot from 2020-10-18 20-01-49

Exclude PRs

Screenshot from 2020-10-18 20-02-39
(as you can see #8 has disappeared)

Select a few Branches

Screenshot from 2020-10-18 20-04-01

Select a few Branches (exclude PRs)

Screenshot from 2020-10-18 20-04-43

Select a few Branches+PRs (Exclude the other PRs)

Screenshot from 2020-10-18 20-05-26

Add backend support for excluding PRs, selecting branches and files.

Fix go-gitea#10327

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath
Copy link
Contributor Author

zeripath commented Sep 8, 2020

Issues:

  • As the selection box expands it begins to float over the top of the graph - this is not ideal.
  • It might be that clicking on the branch should really add it to the selection or have that option.
  • It would also be good to be able to do the graph of the branch roots. (setting ?branch=refs/heads/branchA...refs/heads/branchB does work but it doesn't include the merge base so you get two disconnected branches.)
  • The code just uses a pre-rendered div-only=true section which might be usable elsewhere with some further thought.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 8, 2020
@codecov-commenter
Copy link

codecov-commenter commented Sep 8, 2020

Codecov Report

Merging #12766 into master will increase coverage by 0.00%.
The diff coverage is 52.34%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #12766    +/-   ##
========================================
  Coverage   42.61%   42.62%            
========================================
  Files         671      671            
  Lines       73635    73740   +105     
========================================
+ Hits        31379    31429    +50     
- Misses      37183    37227    +44     
- Partials     5073     5084    +11     
Impacted Files Coverage Δ
modules/context/pagination.go 85.00% <0.00%> (-15.00%) ⬇️
modules/git/repo.go 45.68% <0.00%> (-0.51%) ⬇️
modules/git/repo_commit.go 50.91% <0.00%> (ø)
modules/templates/helper.go 46.13% <0.00%> (-1.13%) ⬇️
routers/repo/commit.go 29.57% <39.39%> (+0.75%) ⬆️
modules/git/ref.go 53.84% <58.33%> (+53.84%) ⬆️
modules/git/commit.go 54.27% <61.53%> (-0.46%) ⬇️
modules/gitgraph/graph.go 57.14% <64.70%> (-3.52%) ⬇️
modules/context/repo.go 55.97% <83.33%> (+0.31%) ⬆️
modules/gitgraph/graph_models.go 86.51% <100.00%> (+2.73%) ⬆️
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54091e0...8616c28. Read the comment docs.

@lunny lunny added the type/enhancement An improvement of existing functionality label Sep 9, 2020
@lunny lunny added this to the 1.14.0 milestone Sep 9, 2020
@bagasme
Copy link
Contributor

bagasme commented Sep 10, 2020

@zeripath Hovering mouse over commit entry in the graph which had branch/PR label cause commit message text half-highlighted:

gitea-highlight-graph

Perhaps try highlighting over <span> text?

@zeripath
Copy link
Contributor Author

you need to reload your css

@silverwind
Copy link
Member

silverwind commented Sep 11, 2020

Header and buttons are not properly vertically centered, Maybe needs some flexbox.

Also, the buttons/dropdown look too big. Maybe try small/tiny classes on them.

Branch tags also have a centering issue, maybe I'll play with those later.

@silverwind
Copy link
Member

silverwind commented Sep 11, 2020

Also, I think using small avatars (20px) might be nice in that list, e.g.

<avatar> <commit title> <tags> <relative time>

@zeripath
Copy link
Contributor Author

zeripath commented Oct 2, 2020

Header and buttons are not properly vertically centered, Maybe needs some flexbox.

Done

Also, the buttons/dropdown look too big. Maybe try small/tiny classes on them.

I can't seem to easily change that.

Branch tags also have a centering issue, maybe I'll play with those later.

I've tried making these display: inline-flex; align-items:center; but I think it looks worse.

Also, I think using small avatars (20px) might be nice in that list, e.g.

<avatar> <commit title> <tags> <relative time>

I think that could be quite expensive to do - it might need another PR.

This does make the whole thing less responsive however

@zeripath
Copy link
Contributor Author

zeripath commented Oct 24, 2020

How about making it:

    li.highlight,
    li.hover {
      background-image: linear-gradient(90deg, rgba(0, 0, 0, .05), transparent 50%);
    }

    li.highlight.hover {
      background-image: linear-gradient(90deg, rgba(0, 0, 0, .1), transparent 50%);
    }

@silverwind
Copy link
Member

Some tweaks in silverwind@c2ce5dc. I made the highlight row-only, I think it's sufficient and not as distracting.

Also noticed the page is missing a title, which should be fixable by adding ctx.Data["Title"] = ctx.Tr("commit_graph") on line 92 of routers/repo/commit.go but for some reason it won't output the translation string for me (maybe translation needs to be activated for that context somewhere?).

@silverwind
Copy link
Member

Some more stuff in silverwind@f633478.

Signed-off-by: Andrew Thornton <[email protected]>
@zeripath
Copy link
Contributor Author

You needed to prefix the commit_graph with repo.

@zeripath
Copy link
Contributor Author

BTW I think the divider between the hide-pr-requests was better - but I'm not going fight over it.

@silverwind
Copy link
Member

silverwind commented Oct 27, 2020

You needed to prefix the commit_graph with repo.

Ah, good to know. I wouldn't have thought of that because the translation key does not have repo..

BTW I think the divider between the hide-pr-requests was better - but I'm not going fight over it.

I think it looked slightly odd odd with non-equal spaced dropdown children.

Oh and one more fix in silverwind@a44ac28ef which fixes some odd z-index issue with the loading indicator and positions it a bit down so it does not stick to the top of the container.

@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, 2020
@lafriks lafriks added the type/changelog Adds the changelog for a new Gitea version label Nov 7, 2020
@lafriks
Copy link
Member

lafriks commented Nov 8, 2020

🚀

@techknowlogick techknowlogick merged commit c05a8ab into go-gitea:master Nov 8, 2020
@zeripath zeripath deleted the fix-10327-commit-graph-options branch November 8, 2020 17:47
@go-gitea go-gitea locked and limited conversation to collaborators Dec 14, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/enhancement An improvement of existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[UI] hide refs/pull/*/head branchs on Commit graph

10 participants