Skip to content

Navbar rework for mobile (repo page and more) #14704

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

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
30 changes: 16 additions & 14 deletions templates/explore/navbar.tmpl
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
<div class="ui secondary pointing tabular top attached borderless stackable menu new-menu navbar">
<a class="{{if .PageIsExploreRepositories}}active{{end}} item" href="{{AppSubUrl}}/explore/repos">
{{svg "octicon-repo"}} {{.i18n.Tr "explore.repos"}}
</a>
<a class="{{if .PageIsExploreUsers}}active{{end}} item" href="{{AppSubUrl}}/explore/users">
{{svg "octicon-person"}} {{.i18n.Tr "explore.users"}}
</a>
<a class="{{if .PageIsExploreOrganizations}}active{{end}} item" href="{{AppSubUrl}}/explore/organizations">
{{svg "octicon-organization"}} {{.i18n.Tr "explore.organizations"}}
</a>
{{if .IsRepoIndexerEnabled}}
<a class="{{if .PageIsExploreCode}}active{{end}} item" href="{{AppSubUrl}}/explore/code">
{{svg "octicon-code"}} {{.i18n.Tr "explore.code"}}
</a>
{{end}}
<div class="new-menu-inner">
<a class="{{if .PageIsExploreRepositories}}active{{end}} item" href="{{AppSubUrl}}/explore/repos">
{{svg "octicon-repo"}} {{.i18n.Tr "explore.repos"}}
</a>
<a class="{{if .PageIsExploreUsers}}active{{end}} item" href="{{AppSubUrl}}/explore/users">
{{svg "octicon-person"}} {{.i18n.Tr "explore.users"}}
</a>
<a class="{{if .PageIsExploreOrganizations}}active{{end}} item" href="{{AppSubUrl}}/explore/organizations">
{{svg "octicon-organization"}} {{.i18n.Tr "explore.organizations"}}
</a>
{{if .IsRepoIndexerEnabled}}
<a class="{{if .PageIsExploreCode}}active{{end}} item" href="{{AppSubUrl}}/explore/code">
{{svg "octicon-code"}} {{.i18n.Tr "explore.code"}}
</a>
{{end}}
</div>
</div>
8 changes: 4 additions & 4 deletions templates/explore/repo_list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@
{{range .Repos}}
<div class="item">
<div class="ui header df ac">
<div class="repo-title">
<div class="repo-title di">
{{$avatar := (repoAvatar . 32 "mr-3")}}
{{if $avatar}}
{{$avatar}}
{{end}}
<a class="name" href="{{.Link}}">
{{if or $.PageIsExplore $.PageIsProfileStarList }}{{if .Owner}}{{.Owner.Name}} / {{end}}{{end}}{{.Name}}
</a>
<div class="labels df ac fw">
{{- if or $.PageIsExplore $.PageIsProfileStarList }}{{if .Owner}}{{.Owner.Name}} / {{end}}{{end}}{{.Name -}}
</a><!--
--><div class="labels dif ac fw vm">
Copy link
Contributor

@zeripath zeripath Feb 23, 2021

Choose a reason for hiding this comment

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

Do you really want the comment emitted here? or do you just want the template not to emit whitespace?

If the latter you may want to use:

{{- /* a comment with white space trimmed from preceding and following text */ -}}

And more usefully you can put your reasoning why this spacing has to be removed as the comment...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I guess that makes sense.

					</a>
					{{-  /* trim whitespace for inline div */ -}}
					<div class="labels dif ac fw vm">

{{if .IsArchived}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.archived"}}</span>
{{end}}
Expand Down
84 changes: 50 additions & 34 deletions templates/repo/header.tmpl
Original file line number Diff line number Diff line change
@@ -1,38 +1,53 @@
<div class="header-wrapper">
{{with .Repository}}
<div class="ui container">
<div class="repo-header">
<div class="repo-header fw">
<div class="repo-title-wrap df fc">
<div class="repo-title">
<div class="repo-title fw">
{{$avatar := (repoAvatar . 32 "mr-3")}}
{{if $avatar}}
{{if $.PageIsSettings}}
<div class="repo-icon mr-3">{{svg "octicon-tools" 32}}</div>
{{else if $avatar}}
{{$avatar}}
{{else}}
{{template "repo/icon" .}}
{{end}}
<a href="{{AppSubUrl}}/{{.Owner.Name}}">{{.Owner.Name}}</a>
<div class="mx-2">/</div>
<a href="{{$.RepoLink}}">{{.Name}}</a>
<div class="labels df ac fw">
{{if .IsTemplate}}
{{if .IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.private_template"}}</span>
{{else}}
{{if .Owner.Visibility.IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.internal_template"}}</span>
<div class="df fw">
<a href="{{AppSubUrl}}/{{.Owner.Name}}">{{.Owner.Name}}</a>
<div class="mx-2">/</div>
</div>
<div class="di">
<a href="{{$.RepoLink}}">{{.Name}}</a>
{{- if $.PageIsSettings -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be helpful to add /* */ within the {{- ... -}} to explain that the {{- is intentional and why it is intentional.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looked around in the code base and found several instances of /* */. I'm not sure whether I need to explain the intention here. The immediate parent 2 lines above clearly says di (display: inline !important;).

Copy link
Contributor

Choose a reason for hiding this comment

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

A lot of our UI stuff is essentially being done by cargo-cult - it's not clear why stuff is needed or why it breaks when you adjust the templates. We need to get better and if there is something unusual it needs to be explained.

If there is a necessary thing like this which could break the UI - it needs to be explicit - otherwise the next person who comes along will add another block in there without the {{- whitespace removal and then wonder why it doesn't work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I understand that. A lot of the spacing is controlled by whitespace (I even saw a &nbsp;&nbsp;&nbsp;), and to me the margin and padding convenience classes work like whitespace (or one might say \hspace from LaTeX) since regular whitespaces are eaten by flexboxes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How do you like this?

Also, found this from you ;-)
whitespace

<div class="di mx-2">/</div><span class="mr-2">{{$.i18n.Tr "repo.settings"}}</span>
{{- else -}}
<div class="labels dif ac fw vm">
{{if .IsTemplate}}
{{if .IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.private_template"}}</span>
{{else}}
{{if .Owner.Visibility.IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.internal_template"}}</span>
{{end}}
{{end}}
{{else}}
{{if .IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.private"}}</span>
{{else}}
{{if .Owner.Visibility.IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.internal"}}</span>
{{end}}
{{end}}
{{end}}
{{end}}
{{else}}
{{if .IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.private"}}</span>
{{else}}
{{if .Owner.Visibility.IsPrivate}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.internal"}}</span>
{{if .IsArchived}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.archived"}}</span>
{{end}}
{{end}}
{{end}}
{{if .IsArchived}}
<span class="ui basic label">{{$.i18n.Tr "repo.desc.archived"}}</span>
{{if $.Permission.IsAdmin}}
<a class="ui compact tiny basic icon button poping up" href="{{$.RepoLink}}/settings" data-content="{{$.i18n.Tr "repo.settings"}}" data-position="right center" data-variation="tiny">
{{svg "octicon-tools"}}
</a>
{{end}}
</div>
{{end}}
</div>
</div>
Expand Down Expand Up @@ -79,9 +94,10 @@
</div><!-- end grid -->
</div><!-- end container -->
{{end}}
{{if not $.PageIsSettings}}
<div class="ui tabs container">
{{if not .Repository.IsBeingCreated}}
<div class="ui tabular stackable menu navbar">
<div class="ui tabular menu navbar">
{{if .Permission.CanRead $.UnitTypeCode}}
<a class="{{if .PageIsViewCode}}active{{end}} item" href="{{.RepoLink}}{{if (ne .BranchName .Repository.DefaultBranch)}}/src/{{.BranchNameSubURL | EscapePound}}{{end}}">
{{svg "octicon-code"}} {{.i18n.Tr "repo.code"}}
Expand All @@ -99,7 +115,7 @@

{{if .Permission.CanRead $.UnitTypeExternalTracker}}
<a class="{{if .PageIsIssueList}}active{{end}} item" href="{{.RepoExternalIssuesLink}}" target="_blank" rel="noopener noreferrer">
{{svg "octicon-link-external"}} {{.i18n.Tr "repo.issues"}} </span>
{{svg "octicon-link-external"}} {{.i18n.Tr "repo.issues"}}
</a>
{{end}}

Expand Down Expand Up @@ -144,15 +160,15 @@

{{template "custom/extra_tabs" .}}

{{if .Permission.IsAdmin}}
<div class="right menu">
<a class="{{if .PageIsSettings}}active{{end}} item" href="{{.RepoLink}}/settings">
{{svg "octicon-tools"}} {{.i18n.Tr "repo.settings"}}
</a>
</div>
{{end}}
<div class="spacer"></div>
</div>
{{end}}
</div>
<div class="ui tabs divider"></div>
<div class="df">
<div class="ui tabs divider f1"></div>
<div class="ui tabs divider container mx-0"></div>
<div class="ui tabs divider spacer"></div>
<div class="ui tabs divider f1"></div>
</div>
{{end}}
</div>
2 changes: 1 addition & 1 deletion templates/repo/settings/navbar.tmpl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="ui secondary pointing tabular top attached borderless menu stackable new-menu navbar shadow-body">
<div class="ui secondary pointing tabular top attached borderless menu stackable new-menu navbar">
<div class="new-menu-inner">
<a class="{{if .PageIsSettingsOptions}}active{{end}} item" href="{{.RepoLink}}/settings">
{{.i18n.Tr "repo.settings.options"}}
Expand Down
128 changes: 73 additions & 55 deletions web_src/less/_base.less
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@
--color-input-border: #dedede;
--color-input-border-hover: #cecece;
--color-navbar: #f8f8f8;
--color-navbar-transparent: #f8f8f800;
--color-light: #00000004;
--color-light-border: #0000001d;
--color-hover: #0000000d;
Expand Down Expand Up @@ -248,12 +249,12 @@ a.muted:hover,

.page-content {
margin-top: 15px;
}

.page-content .header-wrapper,
.page-content .new-menu {
margin-top: -15px !important;
padding-top: 15px !important;
.header-wrapper,
.new-menu {
margin-top: -15px !important;
padding-top: 15px !important;
}
}

.ui.input.focus > input,
Expand Down Expand Up @@ -285,6 +286,11 @@ a.muted:hover,

.ui.menu .item > .label {
background: var(--color-grey);

&:not(.floating) {
margin-left: 7px;
padding: .3em .5em;
}
}

.ui.link.menu .item:hover,
Expand Down Expand Up @@ -1237,46 +1243,33 @@ footer {
margin-bottom: 15px;
background: var(--color-navbar);
border-bottom: 1px solid var(--color-secondary) !important;
overflow: auto;
}
overflow-y: visible;

@media @mediaSm {
.ui.menu.new-menu {
overflow: visible !important;
}
}
.new-menu-inner {
display: flex;
overflow-x: auto;
overflow-y: hidden;
margin-left: auto;
margin-right: auto;

.ui.menu.new-menu .new-menu-inner {
display: flex;
margin-left: auto;
margin-right: auto;
}
@media @mediaSm {
& {
flex-wrap: wrap;
overflow: visible !important;
margin-left: 0;
margin-right: 0;
}
}

@media @mediaSm {
.ui.menu.new-menu .new-menu-inner {
flex-wrap: wrap;
margin-left: 0;
margin-right: 0;
&::before,
&::after {
height: 38px !important;
}
}
}

.ui.menu.new-menu::after {
position: absolute;
display: block;
background: linear-gradient(to right, transparent, var(--color-navbar) 100%);
content: '';
right: 0;
height: 39px;
width: 60px;
visibility: visible;
pointer-events: none;
}

.ui.menu.new-menu.shadow-body::after {
background: linear-gradient(to right, transparent, var(--color-body) 100%);
}

.ui.menu.new-menu .item {
height: 40px;
margin: 0 !important;
}

Expand All @@ -1286,25 +1279,49 @@ footer {
}
}

.ui.menu.new-menu .item:last-child {
padding-right: 30px !important;
}
.ui.tabs.container .ui.menu,
.ui.menu.new-menu .new-menu-inner {
&::before,
&::after {
content: "";
position: absolute;
display: block;
min-width: 1em;
height: 39px;
visibility: visible;
pointer-events: none;
}

.ui.menu.new-menu::-webkit-scrollbar {
height: 6px;
display: none;
}
&::before {
background: linear-gradient(to left, var(--color-navbar-transparent), var(--color-navbar) 75%);
left: 0;
z-index: 1;
}

.ui.menu.new-menu::-webkit-scrollbar-track {
background: none !important;
}
&::after {
background: linear-gradient(to right, var(--color-navbar-transparent), var(--color-navbar) 75%);
right: 0;
}

.ui.menu.new-menu::-webkit-scrollbar-thumb {
box-shadow: none !important;
}
&::-webkit-scrollbar {
height: 10px;
display: none;
}

.ui.menu.new-menu:hover::-webkit-scrollbar {
display: block;
&::-webkit-scrollbar-track {
background: none;
}
/* Exclude Firefox */
@supports (-webkit-appearance:none) and (not (-moz-appearance:none)) {
&:hover {
overflow-x: scroll !important;
margin-bottom: -10px !important;

&::-webkit-scrollbar {
display: block;
}
}
}
}

[v-cloak] {
Expand All @@ -1326,7 +1343,7 @@ footer {
display: flex;
align-items: center;
flex: 1;
word-break: break-all;
word-break: break-word;
color: var(--color-text-light);

.avatar {
Expand All @@ -1335,6 +1352,7 @@ footer {
}

.labels {
margin-top: -.25rem;
margin-left: .5rem;

> * + * {
Expand Down Expand Up @@ -1993,7 +2011,7 @@ table th[data-sortt-desc] {
}

.ui.header .ui.label {
margin-left: .25rem;
margin-left: 0;
}

.ui.header > .ui.label.compact {
Expand Down
5 changes: 1 addition & 4 deletions web_src/less/_explore.less
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,9 @@
font-size: 1.5rem;
margin-bottom: .5rem;

.name {
word-break: break-all;
}

.metas {
font-size: 14px;
align-self: flex-start;
}
}

Expand Down
Loading