Remove (almost) server side data rendering from repo-search component #2317

Morlinest · 2017-08-17T08:04:33Z

Server will be needed only for rendering localization strings.

lunny · 2017-08-17T08:44:33Z

public/js/index.js

@@ -1675,6 +1675,26 @@ function initVueComponents(){
                type: Number,
                required: true
            },
+            orgs: {


Where the variables are evaluated?

All variables are "filled by server" in dashboard.tmpl when creating Vue component and then used inside repo-search.tmpl.

lafriks · 2017-08-17T14:31:43Z

templates/user/dashboard/dashboard.tmpl

+					{{if not .ContextUser.IsOrganization}}
+					:orgs="[
+						{{range .ContextUser.Orgs}}
+						{name: '{{.Name}}', num_repos: '{{.NumRepos}}'},


I think casing numRepos should be used instead of num_repos

The reason for this name is to act like it is loaded from json, where you use (at least in repo api).

ok, than leave it as num_repos

lafriks · 2017-08-17T14:35:12Z

templates/user/dashboard/dashboard.tmpl

+						{{end}}
+					]"
+					:is-org="false"
+					:orgs-total="{{.ContextUser.GetOrganizationCount}}"


is orgs-total really needed (if yes than it should be named as organization-count)? shouldn't it be the same as orgs.length?

Yes, it is. It's same as reposTotal. Now gitea loads everything at once, but if we add pagination (and maybe search) in the future, we don't need to care about (I think repositories can and should be preloaded this way too, instead of making init search).

ok, than anyway it should be called organzation-total-count. Making shorthand variable names does not help readability to understand code later

Also renamed reposTotal to reposTotalCount

lafriks · 2017-08-17T14:36:53Z

templates/user/dashboard/dashboard.tmpl

+						{name: '{{.Name}}', num_repos: '{{.NumRepos}}'},
+						{{end}}
+					]"
+					:is-org="false"


also I think it should be called is-organization

I can rename it, it's just shorthand.

lafriks · 2017-08-17T14:37:15Z

templates/user/dashboard/dashboard.tmpl

+					:uid="uid"
+					:more-repos-link="'{{.ContextUser.HomeLink}}'"
+					{{if not .ContextUser.IsOrganization}}
+					:orgs="[


I think it should be called organizations

I can rename it, it's just shorthand.

please do for sake of code readability

Morlinest · 2017-08-17T15:16:54Z

@lunny @lafriks One question, should I rebase with requested changes or do new commit?

lafriks · 2017-08-17T15:21:15Z

you can do both ways as changes will be squashed later anyway to single commit when merging PR

lafriks · 2017-08-17T16:29:07Z

Please resolve conflicts after #2318

Morlinest · 2017-08-17T17:37:11Z

@lafriks Rebased

lafriks · 2017-08-17T19:07:00Z

@Morlinest force push again to force drone build again

strk · 2017-08-18T21:44:47Z

I gave this a try to check if it fixed #2331 (it did) and noticed a misrendering of help text in repository search input, which goes: Find a repository \.\.\. (with those backslashes)

strk · 2017-08-18T21:45:43Z

Oh, and the plus icon on the top-right corner of that Find a repository block goes to a broken url repo/create> (with the ending closing angle bracket)

Morlinest · 2017-08-19T15:53:49Z

Rebased on master (which includes fix for #2331).

… template

lunny · 2017-08-22T12:56:39Z

LGTM

lafriks · 2017-08-22T12:57:59Z

LGTM

lunny added type/refactoring Existing code has been cleaned up. There should be no new functionality. topic/ui Change the appearance of the Gitea UI labels Aug 17, 2017

lunny added this to the 1.2.0 milestone Aug 17, 2017

lunny reviewed Aug 17, 2017

View reviewed changes

lafriks requested changes Aug 17, 2017

View reviewed changes

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 17, 2017

Morlinest mentioned this pull request Aug 17, 2017

Make use of Vue more universal #2318

Merged

lafriks approved these changes Aug 17, 2017

View reviewed changes

Morlinest force-pushed the improve-repo-search-component branch from 65bfcb6 to bbfef37 Compare August 17, 2017 17:36

Morlinest force-pushed the improve-repo-search-component branch from bbfef37 to 14577d6 Compare August 17, 2017 20:32

Morlinest mentioned this pull request Aug 18, 2017

Go 1.7: {{if}} branches end in different contexts #2331

Closed

Morlinest force-pushed the improve-repo-search-component branch from 14577d6 to edc39c9 Compare August 19, 2017 15:49

Morlinest added 2 commits August 20, 2017 22:29

Remove (almost) server side data rendering from repo-search component…

69865ca

… template

Rename some shorthands

ba5b81a

Morlinest force-pushed the improve-repo-search-component branch from edc39c9 to ba5b81a Compare August 20, 2017 20:30

tboerger added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 22, 2017

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 22, 2017

lunny merged commit 7455604 into go-gitea:master Aug 22, 2017

Morlinest deleted the improve-repo-search-component branch August 22, 2017 14:35

lunny mentioned this pull request Aug 23, 2017

404 when creating new repo #2357

Closed

7 tasks

go-gitea locked and limited conversation to collaborators Nov 23, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove (almost) server side data rendering from repo-search component #2317

Remove (almost) server side data rendering from repo-search component #2317

Morlinest commented Aug 17, 2017

lunny Aug 17, 2017 •

edited

Loading

Morlinest Aug 17, 2017

lafriks Aug 17, 2017

Morlinest Aug 17, 2017

lafriks Aug 17, 2017

lafriks Aug 17, 2017

Morlinest Aug 17, 2017

lafriks Aug 17, 2017

Morlinest Aug 17, 2017

lafriks Aug 17, 2017

Morlinest Aug 17, 2017

lafriks Aug 17, 2017

Morlinest Aug 17, 2017

lafriks Aug 17, 2017

Morlinest commented Aug 17, 2017

lafriks commented Aug 17, 2017

lafriks commented Aug 17, 2017

Morlinest commented Aug 17, 2017

lafriks commented Aug 17, 2017

strk commented Aug 18, 2017

strk commented Aug 18, 2017

Morlinest commented Aug 19, 2017

lunny commented Aug 22, 2017

lafriks commented Aug 22, 2017

Remove (almost) server side data rendering from repo-search component #2317

Remove (almost) server side data rendering from repo-search component #2317

Conversation

Morlinest commented Aug 17, 2017

lunny Aug 17, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Morlinest commented Aug 17, 2017

lafriks commented Aug 17, 2017

lafriks commented Aug 17, 2017

Morlinest commented Aug 17, 2017

lafriks commented Aug 17, 2017

strk commented Aug 18, 2017

strk commented Aug 18, 2017

Morlinest commented Aug 19, 2017

lunny commented Aug 22, 2017

lafriks commented Aug 22, 2017

lunny Aug 17, 2017 •

edited

Loading