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

More tweaks to repo top panel #2267

Merged
merged 4 commits into from
Aug 13, 2017
Merged

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Aug 6, 2017

This includes various tweaks to the repo top panel.

  • Unify the button and clone panel size in the panel and center them better vertically.
  • Hide the green compare button when the comparison would compare same branches, e.g. master..master
  • Remove whitespace around the path so a path is copied as-is and without extra spaces between segments. This results in less readability in the template, but it's necessary. In the long run, minfiying HTML would be a better solution to avoid creation of empty text nodes.

Before

After

</div>
{{ $n := len .TreeNames}}
{{ $l := Subtract $n 1}}
<div class="fitted item"><span class="ui breadcrumb repo-path"><a class="section" href="{{.RepoLink}}/src/{{EscapePound .BranchName}}">{{EllipsisString .Repository.Name 30}}</a>{{range $i, $v := .TreeNames}}<span class="divider">/</span>{{if eq $i $l}}<span class="active section">{{EllipsisString $v 30}}</span>{{else}}{{ $p := index $.Paths $i}}<span class="section"><a href="{{EscapePound $.BranchLink}}/{{EscapePound $p}}">{{EllipsisString $v 30}}</a></span>{{end}}{{end}}</span></div>
Copy link
Member

Choose a reason for hiding this comment

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

This line can be break down to be more humanly readable ^^

Copy link
Member Author

@silverwind silverwind Aug 6, 2017

Choose a reason for hiding this comment

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

Unfortunately not, if we want the path to be copyable as-is. If I add any indentation to this block, a path like someproject/somefile will copy as someproject / somefile or similar to the clipboard. Even adding indentation to the wrapping <div> will result in a trailing whitespace being copied.

As mentioned, a proper fix would be to minify the HTML, maybe by precompiling the templates to a form that does not feature indendation.

@sapk
Copy link
Member

sapk commented Aug 6, 2017

LGTM as this. I add a comment for just a little formating of code.

@tboerger tboerger added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Aug 6, 2017
@lafriks
Copy link
Member

lafriks commented Aug 6, 2017

@silverwind this change breaks integration tests

@silverwind
Copy link
Member Author

silverwind commented Aug 6, 2017

Hmm that's probably because of the removed compare button on master. I'll restore it, didn't realize the compare page allows to switch branches, so it has some use on master too.

@silverwind
Copy link
Member Author

Hmm, still failing. Are you sure it's related?

@lunny
Copy link
Member

lunny commented Aug 7, 2017

It should be related.

@silverwind
Copy link
Member Author

CI is fixed now. It actually was testing for the exact CSS classes.

@andreynering
Copy link
Contributor

LGTM

Nice to see these UI improvements, even small ones. Thank you!

@tboerger tboerger 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 Aug 13, 2017
@andreynering andreynering merged commit fc29a40 into go-gitea:master Aug 13, 2017
@lunny lunny added this to the 1.2.0 milestone Aug 14, 2017
@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Aug 14, 2017
@lunny lunny added the type/enhancement An improvement of existing functionality label Aug 25, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
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. topic/ui Change the appearance of the Gitea UI type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants