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

Prettify number of issues #17760

Merged
merged 21 commits into from
Jun 12, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
7 changes: 4 additions & 3 deletions modules/base/tool.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/setting"
"code.gitea.io/gitea/modules/util"

"github.com/dustin/go-humanize"
)
Expand Down Expand Up @@ -143,9 +144,9 @@ func FileSize(s int64) string {
}

// PrettyNumber produces a string form of the given number in base 10 with
// commas after every three orders of magnitud
func PrettyNumber(v int64) string {
return humanize.Comma(v)
// commas after every three orders of magnitude
func PrettyNumber(i interface{}) string {
return humanize.Comma(util.NumberIntoInt64(i))
lunny marked this conversation as resolved.
Show resolved Hide resolved
}

// Subtract deals with subtraction of all types of number.
Expand Down
1 change: 1 addition & 0 deletions modules/base/tool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ func TestFileSize(t *testing.T) {

func TestPrettyNumber(t *testing.T) {
assert.Equal(t, "23,342,432", PrettyNumber(23342432))
assert.Equal(t, "23,342,432", PrettyNumber(int32(23342432)))
assert.Equal(t, "0", PrettyNumber(0))
assert.Equal(t, "-100,000", PrettyNumber(-100000))
}
Expand Down
34 changes: 22 additions & 12 deletions modules/templates/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"reflect"
"regexp"
"runtime"
"strconv"
"strings"
texttmpl "text/template"
"time"
Expand Down Expand Up @@ -96,18 +97,19 @@ func NewFuncMap() []template.FuncMap {
"CustomEmojis": func() map[string]string {
return setting.UI.CustomEmojisMap
},
"Safe": Safe,
"SafeJS": SafeJS,
"JSEscape": JSEscape,
"Str2html": Str2html,
"TimeSince": timeutil.TimeSince,
"TimeSinceUnix": timeutil.TimeSinceUnix,
"RawTimeSince": timeutil.RawTimeSince,
"FileSize": base.FileSize,
"PrettyNumber": base.PrettyNumber,
"Subtract": base.Subtract,
"EntryIcon": base.EntryIcon,
"MigrationIcon": MigrationIcon,
"Safe": Safe,
"SafeJS": SafeJS,
"JSEscape": JSEscape,
"Str2html": Str2html,
"TimeSince": timeutil.TimeSince,
"TimeSinceUnix": timeutil.TimeSinceUnix,
"RawTimeSince": timeutil.RawTimeSince,
"FileSize": base.FileSize,
"PrettyNumber": base.PrettyNumber,
"JsPrettyNumber": JsPrettyNumber,
"Subtract": base.Subtract,
"EntryIcon": base.EntryIcon,
"MigrationIcon": MigrationIcon,
"Add": func(a ...int) int {
sum := 0
for _, val := range a {
Expand Down Expand Up @@ -928,3 +930,11 @@ func mirrorRemoteAddress(ctx context.Context, m repo_model.RemoteMirrorer) remot

return a
}

// JsPrettyNumber renders a number using english decimal separators, e.g. 1,200 and subsequent
// JS will replace the number with locale-specific separators, based on the user's selected language
func JsPrettyNumber(i interface{}) template.HTML {
num := util.NumberIntoInt64(i)

return template.HTML(`<span class="js-pretty-number" data-value="` + strconv.FormatInt(num, 10) + `">` + base.PrettyNumber(num) + `</span>`)
}
18 changes: 18 additions & 0 deletions modules/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,3 +191,21 @@ var titleCaser = cases.Title(language.English)
func ToTitleCase(s string) string {
return titleCaser.String(s)
}

// NumberIntoInt64 transform a given int into int64.
func NumberIntoInt64(number interface{}) int64 {
var value int64
switch v := number.(type) {
case int:
value = int64(v)
case int8:
value = int64(v)
case int16:
value = int64(v)
case int32:
value = int64(v)
case int64:
value = v
}
return value
}
4 changes: 0 additions & 4 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1256,8 +1256,6 @@ issues.change_ref_at = `changed reference from <b><strike>%s</strike></b> to <b>
issues.remove_ref_at = `removed reference <b>%s</b> %s`
issues.add_ref_at = `added reference <b>%s</b> %s`
issues.delete_branch_at = `deleted branch <b>%s</b> %s`
issues.open_tab = %d Open
issues.close_tab = %d Closed
issues.filter_label = Label
issues.filter_label_exclude = `Use <code>alt</code> + <code>click/enter</code> to exclude labels`
issues.filter_label_no_select = All labels
Expand Down Expand Up @@ -1613,8 +1611,6 @@ pulls.pull_request_scheduled_auto_merge = `scheduled this pull request to auto m
pulls.pull_request_canceled_scheduled_auto_merge = `canceled auto merging this pull request when all checks succeed %[1]s`

milestones.new = New Milestone
milestones.open_tab = %d Open
milestones.close_tab = %d Closed
milestones.closed = Closed %s
milestones.update_ago = Updated %s ago
milestones.no_due_date = No due date
Expand Down
12 changes: 7 additions & 5 deletions templates/repo/issue/milestones.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
<div class="ui compact tiny menu">
<a class="item{{if not .IsShowClosed}} active{{end}}" href="{{.RepoLink}}/milestones?state=open&q={{$.Keyword}}">
{{svg "octicon-milestone" 16 "mr-3"}}
{{.i18n.Tr "repo.milestones.open_tab" .OpenCount}}
{{JsPrettyNumber .OpenCount}}&nbsp;{{.i18n.Tr "repo.issues.open_title"}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Here and below, I worry about i18n here, Open and %d Open may need different translations.

Copy link
Member

Choose a reason for hiding this comment

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

Some languages like Arabic do <open> <num> but we can not indicate num as decimal because it's going to be pre-formatted as string. I guess I can move this to a generic numopen translation token with that string but it's probably going to require re-translation according to earlier comment.

</a>
<a class="item{{if .IsShowClosed}} active{{end}}" href="{{.RepoLink}}/milestones?state=closed&q={{$.Keyword}}">
{{svg "octicon-milestone" 16 "mr-3"}}
{{.i18n.Tr "repo.milestones.close_tab" .ClosedCount}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .ClosedCount}}&nbsp;{{.i18n.Tr "repo.issues.closed_title"}}
</a>
</div>
</div>
Expand Down Expand Up @@ -83,8 +83,10 @@
{{end}}
{{end}}
<span class="issue-stats">
{{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.issues.open_tab" .NumOpenIssues}}
{{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.issues.close_tab" .NumClosedIssues}}
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{JsPrettyNumber .NumOpenIssues}}&nbsp;{{$.i18n.Tr "repo.issues.open_title"}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .NumClosedIssues}}&nbsp;{{$.i18n.Tr "repo.issues.closed_title"}}
{{if .TotalTrackedTime}}{{svg "octicon-clock"}} {{.TotalTrackedTime|Sec2Time}}{{end}}
{{if .UpdatedUnix}}{{svg "octicon-clock"}} {{$.i18n.Tr "repo.milestones.update_ago" (.TimeSinceUpdate|Sec2Time)}}{{end}}
</span>
Expand Down
12 changes: 8 additions & 4 deletions templates/repo/issue/openclose.tmpl
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
<div class="ui compact tiny menu">
<a class="{{if not .IsShowClosed}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type={{$.ViewType}}&sort={{$.SortType}}&state=open&labels={{.SelectLabels}}&milestone={{.MilestoneID}}&assignee={{.AssigneeID}}">
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{.i18n.Tr "repo.issues.open_tab" .IssueStats.OpenCount}}
{{if .PageIsPullList}}
{{svg "octicon-git-pull-request" 16 "mr-3"}}
{{else}}
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{end}}
{{JsPrettyNumber .IssueStats.OpenCount}}&nbsp;{{.i18n.Tr "repo.issues.open_title"}}
</a>
<a class="{{if .IsShowClosed}}active{{end}} item" href="{{$.Link}}?q={{$.Keyword}}&type={{.ViewType}}&sort={{$.SortType}}&state=closed&labels={{.SelectLabels}}&milestone={{.MilestoneID}}&assignee={{.AssigneeID}}">
{{svg "octicon-issue-closed" 16 "mr-3"}}
{{.i18n.Tr "repo.issues.close_tab" .IssueStats.ClosedCount}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .IssueStats.ClosedCount}}&nbsp;{{.i18n.Tr "repo.issues.closed_title"}}
</a>
</div>
14 changes: 8 additions & 6 deletions templates/repo/projects/list.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,12 @@
{{template "base/alert" .}}
<div class="ui compact tiny menu">
<a class="item{{if not .IsShowClosed}} active{{end}}" href="{{.RepoLink}}/projects?state=open">
{{svg "octicon-project" 16 "mr-2"}}
{{.i18n.Tr "repo.issues.open_tab" .OpenCount}}
{{svg "octicon-project" 16 "mr-3"}}
{{JsPrettyNumber .OpenCount}}&nbsp;{{.i18n.Tr "repo.issues.open_title"}}
</a>
<a class="item{{if .IsShowClosed}} active{{end}}" href="{{.RepoLink}}/projects?state=closed">
{{svg "octicon-check" 16 "mr-2"}}
{{.i18n.Tr "repo.milestones.close_tab" .ClosedCount}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .ClosedCount}}&nbsp;{{.i18n.Tr "repo.issues.closed_title"}}
</a>
</div>

Expand Down Expand Up @@ -47,8 +47,10 @@
{{svg "octicon-clock"}} {{$.i18n.Tr "repo.milestones.closed" $closedDate|Str2html}}
{{end}}
<span class="issue-stats">
{{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.issues.open_tab" .NumOpenIssues}}
{{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.issues.close_tab" .NumClosedIssues}}
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{JsPrettyNumber .NumOpenIssues}}&nbsp;{{$.i18n.Tr "repo.issues.open_title"}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .NumClosedIssues}}&nbsp;{{$.i18n.Tr "repo.issues.closed_title"}}
</span>
</div>
{{if and (or $.CanWriteIssues $.CanWritePulls) (not $.Repository.IsArchived)}}
Expand Down
4 changes: 2 additions & 2 deletions templates/user/dashboard/issues.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,11 @@
<div class="ui compact tiny menu">
<a class="item{{if not .IsShowClosed}} active{{end}}" href="{{.Link}}?type={{$.ViewType}}&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state=open&q={{$.Keyword}}">
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{.i18n.Tr "repo.issues.open_tab" .IssueStats.OpenCount}}
{{JsPrettyNumber .ShownIssueStats.OpenCount}}&nbsp;{{.i18n.Tr "repo.issues.open_title"}}
</a>
<a class="item{{if .IsShowClosed}} active{{end}}" href="{{.Link}}?type={{$.ViewType}}&repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state=closed&q={{$.Keyword}}">
{{svg "octicon-issue-closed" 16 "mr-3"}}
{{.i18n.Tr "repo.issues.close_tab" .IssueStats.ClosedCount}}
{{JsPrettyNumber .ShownIssueStats.ClosedCount}}&nbsp;{{.i18n.Tr "repo.issues.closed_title"}}
</a>
</div>
</div>
Expand Down
18 changes: 11 additions & 7 deletions templates/user/dashboard/milestones.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@
<div class="column">
<div class="ui compact tiny menu">
<a class="item{{if not .IsShowClosed}} active{{end}}" href="{{.Link}}?repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state=open&q={{$.Keyword}}">
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{.i18n.Tr "repo.milestones.open_tab" .MilestoneStats.OpenCount}}
{{svg "octicon-milestone" 16 "mr-3"}}
{{JsPrettyNumber .MilestoneStats.OpenCount}}&nbsp;{{.i18n.Tr "repo.issues.open_title"}}
</a>
<a class="item{{if .IsShowClosed}} active{{end}}" href="{{.Link}}?repos=[{{range $.RepoIDs}}{{.}}%2C{{end}}]&sort={{$.SortType}}&state=closed&q={{$.Keyword}}">
{{svg "octicon-issue-closed" 16 "mr-3"}}
{{.i18n.Tr "repo.milestones.close_tab" .MilestoneStats.ClosedCount}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .MilestoneStats.ClosedCount}}&nbsp;{{.i18n.Tr "repo.issues.closed_title"}}
</a>
</div>
</div>
Expand Down Expand Up @@ -103,9 +103,13 @@
{{end}}
{{end}}
<span class="issue-stats">
{{svg "octicon-issue-opened"}} {{$.i18n.Tr "repo.milestones.open_tab" .NumOpenIssues}}
{{svg "octicon-issue-closed"}} {{$.i18n.Tr "repo.milestones.close_tab" .NumClosedIssues}}
{{if .TotalTrackedTime}}{{svg "octicon-clock"}} {{.TotalTrackedTime|Sec2Time}}{{end}}
{{svg "octicon-issue-opened" 16 "mr-3"}}
{{JsPrettyNumber .NumOpenIssues}}&nbsp;{{$.i18n.Tr "repo.issues.open_title"}}
{{svg "octicon-check" 16 "mr-3"}}
{{JsPrettyNumber .NumClosedIssues}}&nbsp;{{$.i18n.Tr "repo.issues.closed_title"}}
{{if .TotalTrackedTime}}
{{svg "octicon-clock"}} {{.TotalTrackedTime|Sec2Time}}
{{end}}
</span>
</div>
{{if and (or $.CanWriteIssues $.CanWritePulls) (not $.Repository.IsArchived)}}
Expand Down
14 changes: 14 additions & 0 deletions web_src/js/features/formatting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import {prettyNumber} from '../utils.js';

const {lang} = document.documentElement;
Gusted marked this conversation as resolved.
Show resolved Hide resolved

export function initFormattingReplacements() {
// replace english formatted numbers with locale-specific separators
for (const el of document.getElementsByClassName('js-pretty-number')) {
const num = Number(el.getAttribute('data-value'));
const formatted = prettyNumber(num, lang);
if (formatted && formatted !== el.textContent) {
el.textContent = formatted;
}
}
}
5 changes: 5 additions & 0 deletions web_src/js/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ import {initRepoBranchButton} from './features/repo-branch.js';
import {initCommonOrganization} from './features/common-organization.js';
import {initRepoWikiForm} from './features/repo-wiki.js';
import {initRepoCommentForm, initRepository} from './features/repo-legacy.js';
import {initFormattingReplacements} from './features/formatting.js';

// Run time-critical code as soon as possible. This is safe to do because this
// script appears at the end of <body> and rendered HTML is accessible at that point.
initFormattingReplacements();

// Silence fomantic's error logging when tabs are used without a target content element
$.fn.tab.settings.silent = true;
Expand Down
7 changes: 7 additions & 0 deletions web_src/js/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,10 @@ export function parseIssueHref(href) {
const [_, owner, repo, type, index] = /([^/]+)\/([^/]+)\/(issues|pulls)\/([0-9]+)/.exec(path) || [];
return {owner, repo, type, index};
}

// pretty-print a number using locale-specific separators, e.g. 1200 -> 1,200
export function prettyNumber(num, locale = 'en-US') {
if (typeof num !== 'number') return '';
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. locale = 'en-US' seems not a good idea, it's better to use browser default.
  2. typeof num !== 'number', maybe we had better support type-casting here, to support string value '123'

Copy link
Member

@silverwind silverwind Nov 28, 2021

Choose a reason for hiding this comment

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

  1. The default is never going to be hit because locale is passed based on HTML attribute. It seemed more efficient to only read the HTML attribute once instead of every call to this function.
  2. I think I'll move it to inline script, so there won't be a function wrapper anymore

const {format} = new Intl.NumberFormat(locale);
return format(num);
}
12 changes: 12 additions & 0 deletions web_src/js/utils.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
basename, extname, isObject, uniq, stripTags, joinPaths, parseIssueHref,
prettyNumber,
} from './utils.js';

test('basename', () => {
Expand Down Expand Up @@ -84,3 +85,14 @@ test('parseIssueHref', () => {
expect(parseIssueHref('https://example.com/sub/sub2/owner/repo/issues/1#hash')).toEqual({owner: 'owner', repo: 'repo', type: 'issues', index: '1'});
expect(parseIssueHref('')).toEqual({owner: undefined, repo: undefined, type: undefined, index: undefined});
});

test('prettyNumber', () => {
expect(prettyNumber()).toEqual('');
expect(prettyNumber(null)).toEqual('');
expect(prettyNumber(undefined)).toEqual('');
expect(prettyNumber('1200')).toEqual('');
expect(prettyNumber(12345678, 'en-US')).toEqual('12,345,678');
expect(prettyNumber(12345678, 'de-DE')).toEqual('12.345.678');
expect(prettyNumber(12345678, 'be-BE')).toEqual('12 345 678');
expect(prettyNumber(12345678, 'hi-IN')).toEqual('1,23,45,678');
});