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

WIP: Further improvements to Git Graph #12209

Closed
wants to merge 1 commit into from

Conversation

zeripath
Copy link
Contributor

  • Restructure to extract out Flow generation and allow flows to be associated with commits
  • Provide an alternative flow generation that prefers trunks
    • algorithm
    • provide a switch
  • Provide a default palette to make colors more predictable
  • Provide a way of pinning colors over page turns.
    • When assigning colors to flows allow colors to be preassigned
    • Pass the colors on page turn
  • It would be helpful if you could highlight commits/flows on mouseover of flows/commits

Signed-off-by: Andrew Thornton art27@cantab.net

Signed-off-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath added topic/ui Change the appearance of the Gitea UI pr/wip This PR is not ready for review labels Jul 10, 2020
@silverwind
Copy link
Member

silverwind commented Jul 11, 2020

Provide a way of pinning colors over page turns.
Pass the colors on page turn

This seems like a overly complicated approach. I would use something like https://github.com/zenozeng/color-hash to convert a branch name to a static color. On dark theme (use isDarkTheme()) add a higher lightness than on light theme to ensure sufficient contrast over the background.

I also once made a personal fork of color-hash with an option to specificy min/max lightness and saturation, maybe it'll be needed.

Also see #10217 (comment)

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jul 11, 2020
@zeripath
Copy link
Contributor Author

Git branches are misnamed they are simply named commits - once a commit has been made there is no way to determine which of the parents had the branch name, or of any of them had it.

It would be lovely if we could apply fixed names to flows. There is no such naming system possible.

So as far as I can see this is the simplest approach.

@zeripath
Copy link
Contributor Author

Even determining whether to create a new flow or end a flow is not even fixed:

Given the following glyphs:

 |    or    \|
/|           |

Which flow continues, which flow creates a new flow?

If we want pinning we will also have to apply #12142 - fortunately that appears to have no detrimental effects on my test machine although it definitely seems wasteful.

@zeripath
Copy link
Contributor Author

But I think you're right in another way flow determination will have to go to the server for the same reason as #12142

@silverwind
Copy link
Member

silverwind commented Jul 11, 2020

Regarding colors, I just suggest a static list of 16 (or more if you like, but the more there are, the less distinction is possible) predefined colors, see #10217 (comment). It's the simplest way and we can tweak the colors to always look good.

@zeripath zeripath closed this Jul 31, 2020
zeripath added a commit that referenced this pull request Aug 6, 2020
Rendering the git graph on the server means that we can properly track flows and switch from the Canvas implementation to a SVG implementation.

* This implementation provides a 16 limited color selection
* The uniqued color numbers are also provided
* And there is also a monochrome version
*In addition is a hover highlight that allows users to highlight commits on the same flow.

Closes #12209

Signed-off-by: Andrew Thornton art27@cantab.net
Co-authored-by: silverwind <me@silverwind.io>
@zeripath zeripath deleted the gitgraph-pinning branch August 6, 2020 17:50
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants