lunny/gitea
Archived
1
1
Fork 1
Code Issues 1.7k Pull Requests 154 Projects Releases 128 Wiki Activity

Frontend refactor: move Vue related code from index.js to components dir, and remove unused codes. #17301

Merged
lunny merged 14 commits from frontend-refactor into main 2021-10-15 02:35:27 +00:00
Conversation 0 Commits 14 Files Changed 20 +718 -634
wxiaoguang commented 2021-10-13 17:54:13 +00:00
Contributor
Copy Link

This is the first big frontend refactor for Frontend Plan (https://github.com/go-gitea/gitea/issues/17149)

Major changes:

  • A guideline document is added
  • Remove unused tags from head.tmpl, including:
    • <head data-suburl="{{AppSubUrl}}">
    • <meta name="_uid" content="{{.SignedUser.ID}}" />
    • <meta name="_context_uid" content="{{.ContextUser.ID}}" />
    • <meta name="_search_limit" content="{{.SearchLimit}}" />
  • Remove unused code submitDeleteForm from repo/view_file.tmpl
  • Move all Vue related code from index.js to components dir, including:
    • DashboardRepoList.js: used by the repo list on the home dashboard
    • RepoActivityTopAuthors.vue, used by /org/repo/activity to show a bar chart for top activity users
    • RepoBranchTagDropdown.js, used by repo pages to show a dropdown list to select branches and tags.
  • Remove most ignorePatterns items from .eslint, we are clear enough now!
This is the first big frontend refactor for Frontend Plan (https://github.com/go-gitea/gitea/issues/17149) Major changes: * A guideline document is added * Remove unused tags from `head.tmpl`, including: * `<head data-suburl="{{AppSubUrl}}">` * `<meta name="_uid" content="{{.SignedUser.ID}}" />` * `<meta name="_context_uid" content="{{.ContextUser.ID}}" />` * `<meta name="_search_limit" content="{{.SearchLimit}}" />` * Remove unused code `submitDeleteForm` from `repo/view_file.tmpl` * Move all Vue related code from `index.js` to `components` dir, including: * `DashboardRepoList.js`: used by the repo list on the home dashboard * `RepoActivityTopAuthors.vue`, used by `/org/repo/activity` to show a bar chart for top activity users * `RepoBranchTagDropdown.js`, used by repo pages to show a dropdown list to select branches and tags. * Remove most `ignorePatterns` items from `.eslint`, we are clear enough now!
delvh reviewed 2021-10-13 21:39:41 +00:00
docs/content/doc/developers/guidelines-frontend.md
delvh commented 2021-10-13 20:21:58 +00:00
Contributor 
### Gitea specific guidelines:
```suggestion ### Gitea specific guidelines: ```
delvh commented 2021-10-13 20:22:40 +00:00
Contributor 
1. Every feature (Fomantic-UI/jQuery module) should be put in separate files/ directories.
```suggestion 1. Every feature (Fomantic-UI/jQuery module) should be put in separate files/ directories. ```
delvh commented 2021-10-13 20:26:31 +00:00
Contributor 
2. HTML ids and classes should use kebab-case.
3. HTML ids and classes used in JavaScript should be unique for the whole project, and should contain 2-3 feature related keywords. We recommend to use the `js-` prefix for classes that are only used in JavaScript.
```suggestion 2. HTML ids and classes should use kebab-case. 3. HTML ids and classes used in JavaScript should be unique for the whole project, and should contain 2-3 feature related keywords. We recommend to use the `js-` prefix for classes that are only used in JavaScript. ```
delvh commented 2021-10-13 20:28:37 +00:00
Contributor 
5. CSS styling for classes provided by frameworks should not be overwritten. Always use new class-names to overwrite framework styles. We recommend to use the `us-` prefix for user defined styles.
```suggestion 5. CSS styling for classes provided by frameworks should not be overwritten. Always use new class-names to overwrite framework styles. We recommend to use the `us-` prefix for user defined styles. ```
delvh commented 2021-10-13 20:29:29 +00:00
Contributor 
6. The backend can pass complex data to the frontend by using `ctx.PageData["myModuleData"] = map{}`
```suggestion 6. The backend can pass complex data to the frontend by using `ctx.PageData["myModuleData"] = map{}` ```
delvh commented 2021-10-13 20:31:15 +00:00
Contributor 
### Too much code in `web_src/index.js`
```suggestion ### Too much code in `web_src/index.js` ```
delvh commented 2021-10-13 20:32:39 +00:00
Contributor 
Previously, most JavaScript code was written into `web_src/index.js` directly, making the file unmaintainable.
Try to keep this file small by creating new modules instead. These modules can be put in the `web_src/js/features` directory for now.
```suggestion Previously, most JavaScript code was written into `web_src/index.js` directly, making the file unmaintainable. Try to keep this file small by creating new modules instead. These modules can be put in the `web_src/js/features` directory for now. ```
delvh commented 2021-10-13 20:33:05 +00:00
Contributor 
7. Simple pages and SEO-related pages use the Go Text Template render to generate static Fomantic-UI HTML output. Complex pages can use Vue2 (or Vue3 in future).
```suggestion 7. Simple pages and SEO-related pages use the Go Text Template render to generate static Fomantic-UI HTML output. Complex pages can use Vue2 (or Vue3 in future). ```
delvh commented 2021-10-13 20:34:46 +00:00
Contributor 
Gitea is using Vue2 now, we plan to upgrade to Vue3. We decided not to introduce JSX to keep the HTML and the JavaScript code separated.
```suggestion Gitea is using Vue2 now, we plan to upgrade to Vue3. We decided not to introduce JSX to keep the HTML and the JavaScript code separated. ```
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +21,4 @@
## Background
Gitea uses [Less CSS](https://lesscss.org), [Fomantic-UI](https://fomantic-ui.com/introduction/getting-started.html) (based on [jQuery](https://api.jquery.com)) and [Vue2](https://vuejs.org/v2/guide/) for its frontend.
delvh commented 2021-10-13 20:20:53 +00:00
Contributor 
Gitea uses [Less CSS](https://lesscss.org), [Fomantic-UI](https://fomantic-ui.com/introduction/getting-started.html) (based on [jQuery](https://api.jquery.com)), and [Vue2](https://vuejs.org/v2/guide/) for its frontend.
```suggestion Gitea uses [Less CSS](https://lesscss.org), [Fomantic-UI](https://fomantic-ui.com/introduction/getting-started.html) (based on [jQuery](https://api.jquery.com)), and [Vue2](https://vuejs.org/v2/guide/) for its frontend. ```
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +27,4 @@
## General Guidelines
We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
delvh commented 2021-10-13 20:21:19 +00:00
Contributor 
We recommend the [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
```suggestion We recommend the [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html) ```
docs/content/doc/developers/hacking-on-gitea.en-us.md
delvh commented 2021-10-13 20:37:39 +00:00
Contributor 
Frontend development should follow the [Guidelines for Frontend Development](./guidelines-frontend.md)

Of course, you could also just build once:
```suggestion Frontend development should follow the [Guidelines for Frontend Development](./guidelines-frontend.md) Of course, you could also just build once: ```
routers/web/repo/activity.go
@ -62,3 +62,3 @@
if ctx.Data["ActivityTopAuthors"], err = models.GetActivityStatsTopAuthors(ctx.Repo.Repository, timeFrom, 10); err != nil {
if ctx.PageData["repoActivityTopAuthors"], err = models.GetActivityStatsTopAuthors(ctx.Repo.Repository, timeFrom, 10); err != nil {
ctx.ServerError("GetActivityStatsTopAuthors", err)
delvh commented 2021-10-13 20:42:19 +00:00
Contributor

Do we use UpperCamelCase for ctx.Data and lowerCamelCase for ctx.PageData? Why?
Could we please be consistent here, or would that lead to UpperCamelCase variables in the frontend?

Do we use UpperCamelCase for `ctx.Data` and lowerCamelCase for `ctx.PageData`? Why? Could we please be consistent here, or would that lead to UpperCamelCase variables in the frontend?
routers/web/user/home.go
@ -110,1 +110,4 @@
ctx.PageData["dashboardRepoList"] = map[string]interface{}{
"searchLimit": setting.UI.User.RepoPagingNum,
}
delvh commented 2021-10-13 20:43:39 +00:00
Contributor

😕
What's the benefit in this case of using a single-entry map instead of a simple int?

:confused: What's the benefit in this case of using a single-entry map instead of a simple int?
templates/base/head.tmpl
@ -1,6 +1,6 @@
<!DOCTYPE html>
<html lang="{{.Lang}}" class="theme-{{.SignedUser.Theme}}">
delvh commented 2021-10-13 20:53:36 +00:00
Contributor

I don't think that will form a valid html file. Was that taken out by accident? Did you perhaps simply want to remove the data-suburl?

<html lang="{{.Lang}}" class="theme-{{.SignedUser.Theme}}">
<head>

Alternatively, the original line:

<html lang="{{.Lang}}" class="theme-{{.SignedUser.Theme}}">
<head data-suburl="{{AppSubUrl}}">
I don't think that will form a valid html file. Was that taken out by accident? Did you perhaps simply want to remove the `data-suburl`? ```suggestion <html lang="{{.Lang}}" class="theme-{{.SignedUser.Theme}}"> <head> ``` Alternatively, the original line: ```suggestion <html lang="{{.Lang}}" class="theme-{{.SignedUser.Theme}}"> <head data-suburl="{{AppSubUrl}}"> ```
templates/base/head.tmpl
@ -36,3 +28,3 @@
csrf: '{{.CsrfToken}}',
PageData: {{ .PageData }},
pageData: {{ .PageData }},
HighlightJS: {{if .RequireHighlightJS}}true{{else}}false{{end}},
delvh commented 2021-10-13 20:56:01 +00:00
Contributor

Were those extracted to ctx.PageData and are hence not needed anymore?

Were those extracted to `ctx.PageData` and are hence not needed anymore?
templates/repo/activity.tmpl
delvh commented 2021-10-13 20:57:19 +00:00
Contributor 
					<div id="repo-activity-top-authors-chart" class="ui attached segment"></div>
```suggestion <div id="repo-activity-top-authors-chart" class="ui attached segment"></div> ```
templates/repo/view_file.tmpl
delvh commented 2021-10-13 21:27:57 +00:00
Contributor

What happened to this function? Why could it be deleted (as it seems without replacement as far as I can see)?

What happened to this function? Why could it be deleted (as it seems without replacement as far as I can see)?
templates/user/dashboard/repolist.tmpl
@ -1,8 +1,7 @@
<div id="app" class="six wide column">
<div id="dashboard-repo-list" class="six wide column">
<repo-search
delvh commented 2021-10-13 21:00:44 +00:00
Contributor

Assuming it still works: 👍

Assuming it still works: :+1:
web_src/js/components/ActivityHeatmap.vue
@ -70,4 +70,3 @@ export default {
},
};
</script>
<style scoped/>
delvh commented 2021-10-13 21:02:27 +00:00
Contributor

You now only require this file when you are on the correct page and hence removed the scoped attribute?

You now only require this file when you are on the correct page and hence removed the `scoped` attribute?
web_src/js/components/DashboardRepoList.js
@ -0,0 +26,4 @@
},
organizations: {
type: Array,
default: () => [],
delvh commented 2021-10-13 21:04:22 +00:00
Contributor 
        default: [],

Or why would you use an additional function call?

```suggestion default: [], ``` Or why would you use an additional function call?
web_src/js/components/DashboardRepoList.js
@ -0,0 +30,4 @@
},
isOrganization: {
type: Boolean,
default: true
delvh commented 2021-10-13 21:05:33 +00:00
Contributor 
        default: false

Why would you assume by default that our user is an organization?

```suggestion default: false ``` Why would you assume by default that our user is an organization?
web_src/js/components/DashboardRepoList.js
@ -0,0 +252,4 @@
break;
default:
this.archivedFilter = 'unarchived';
break;
delvh commented 2021-10-13 21:31:03 +00:00
Contributor 
          case 'unarchived':
            this.archivedFilter = 'archived';
            break;
          case 'archived':
            this.archivedFilter = 'both';
            break;
          case 'both':
          default:
            this.archivedFilter = 'unarchived';
            break;
```suggestion case 'unarchived': this.archivedFilter = 'archived'; break; case 'archived': this.archivedFilter = 'both'; break; case 'both': default: this.archivedFilter = 'unarchived'; break; ```
web_src/js/components/DashboardRepoList.js
@ -0,0 +274,4 @@
break;
default:
this.privateFilter = 'both';
break;
delvh commented 2021-10-13 21:31:45 +00:00
Contributor 
          case 'private':
          default:
            this.privateFilter = 'both';
            break;
```suggestion case 'private': default: this.privateFilter = 'both'; break; ```
web_src/js/components/DashboardRepoList.js
@ -0,0 +291,4 @@
}
if (this.page < 1) {
this.page = 1;
}
delvh commented 2021-10-13 21:34:11 +00:00
Contributor 
        this.page = Math.min(page, this.finalPage);
        this.page = Math.max(this.page, 1);
```suggestion this.page = Math.min(page, this.finalPage); this.page = Math.max(this.page, 1); ```
web_src/js/components/VueComponentLoader.js
@ -0,0 +6,4 @@
let vueEnvInited = false;
function initVueEnv() {
if (vueEnvInited) return;
vueEnvInited = true;
delvh commented 2021-10-13 21:20:16 +00:00
Contributor 
let vueEnvInitialized = false;
function initVueEnv() {
  if (vueEnvInitialized) return;
  vueEnvInitialized = true;
```suggestion let vueEnvInitialized = false; function initVueEnv() { if (vueEnvInitialized) return; vueEnvInitialized = true; ```
web_src/js/components/VueComponentLoader.js
@ -0,0 +16,4 @@
let vueSvgInited = false;
function initVueSvg() {
if (vueSvgInited) return;
vueSvgInited = true;
delvh commented 2021-10-13 21:20:39 +00:00
Contributor 
let vueSvgInitialized = false;
function initVueSvg() {
  if (vueSvgInitialized) return;
  vueSvgInitialized = true;
```suggestion let vueSvgInitialized = false; function initVueSvg() { if (vueSvgInitialized) return; vueSvgInitialized = true; ```
wxiaoguang reviewed 2021-10-14 01:03:40 +00:00
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +21,4 @@
## Background
Gitea uses [Less CSS](https://lesscss.org), [Fomantic-UI](https://fomantic-ui.com/introduction/getting-started.html) (based on [jQuery](https://api.jquery.com)) and [Vue2](https://vuejs.org/v2/guide/) for its frontend.
wxiaoguang commented 2021-10-14 01:03:40 +00:00
Author
Contributor

It's not a syntax problem. See: https://www.grammarly.com/blog/comma-before-and/

It's not a syntax problem. See: https://www.grammarly.com/blog/comma-before-and/
wxiaoguang reviewed 2021-10-14 01:04:42 +00:00
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +27,4 @@
## General Guidelines
We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
wxiaoguang commented 2021-10-14 01:04:41 +00:00
Author
Contributor

It's not a syntax problem: https://english.stackexchange.com/questions/273067/when-to-use-the-in-book-titles

It's not a syntax problem: https://english.stackexchange.com/questions/273067/when-to-use-the-in-book-titles
wxiaoguang reviewed 2021-10-14 01:06:34 +00:00
docs/content/doc/developers/guidelines-frontend.md
wxiaoguang commented 2021-10-14 01:06:34 +00:00
Author
Contributor

In my mind, this list is supplementary clauses to the google guidelines, so we do not need a new section.

In my mind, this list is supplementary clauses to the google guidelines, so we do not need a new section.
wxiaoguang reviewed 2021-10-14 01:09:14 +00:00
docs/content/doc/developers/guidelines-frontend.md
wxiaoguang commented 2021-10-14 01:09:14 +00:00
Author
Contributor

As above, I think we do not call the Vue2 or the Go Text Template.

As above, I think we do not call `the Vue2` or `the Go Text Template`.
lunny reviewed 2021-10-14 01:10:25 +00:00
docs/content/doc/developers/guidelines-frontend.md
lunny commented 2021-10-14 01:10:25 +00:00
Owner 
The HTML pages are rendered by [Go HTML Template](https://pkg.go.dev/html/template)
```suggestion The HTML pages are rendered by [Go HTML Template](https://pkg.go.dev/html/template) ```
wxiaoguang reviewed 2021-10-14 01:10:37 +00:00
docs/content/doc/developers/hacking-on-gitea.en-us.md
wxiaoguang commented 2021-10-14 01:10:37 +00:00
Author
Contributor

watch-frontend is better for development so we had better keep it here

`watch-frontend` is better for development so we had better keep it here
wxiaoguang reviewed 2021-10-14 01:13:11 +00:00
routers/web/repo/activity.go
@ -62,3 +62,3 @@
if ctx.Data["ActivityTopAuthors"], err = models.GetActivityStatsTopAuthors(ctx.Repo.Repository, timeFrom, 10); err != nil {
if ctx.PageData["repoActivityTopAuthors"], err = models.GetActivityStatsTopAuthors(ctx.Repo.Repository, timeFrom, 10); err != nil {
ctx.ServerError("GetActivityStatsTopAuthors", err)
wxiaoguang commented 2021-10-14 01:13:11 +00:00
Author
Contributor

Most keys in window.config use PascalCase. Some people prefer to use camelCase for (new) JavaScript keys.

Both are fine to me.

Most keys in `window.config` use PascalCase. Some people prefer to use camelCase for (new) JavaScript keys. Both are fine to me.
wxiaoguang reviewed 2021-10-14 01:14:55 +00:00
routers/web/user/home.go
@ -110,1 +110,4 @@
ctx.PageData["dashboardRepoList"] = map[string]interface{}{
"searchLimit": setting.UI.User.RepoPagingNum,
}
wxiaoguang commented 2021-10-14 01:14:55 +00:00
Author
Contributor

More options can be added in future. And PageData passes data for JavaScript modeuls like ctx.PageData["theModuleName"], so we can not just put an int here.

More options can be added in future. And `PageData` passes data for JavaScript modeuls like `ctx.PageData["theModuleName"]`, so we can not just put an int here.
wxiaoguang reviewed 2021-10-14 01:16:26 +00:00
templates/base/head.tmpl
@ -36,3 +28,3 @@
csrf: '{{.CsrfToken}}',
PageData: {{ .PageData }},
pageData: {{ .PageData }},
HighlightJS: {{if .RequireHighlightJS}}true{{else}}false{{end}},
wxiaoguang commented 2021-10-14 01:16:26 +00:00
Author
Contributor

Yes, I searched the whole project, only legacy dashboard repo list depends on <meta name="_search_limit"

Yes, I searched the whole project, only legacy `dashboard repo list` depends on `<meta name="_search_limit"`
wxiaoguang reviewed 2021-10-14 01:19:39 +00:00
templates/repo/activity.tmpl
wxiaoguang commented 2021-10-14 01:19:39 +00:00
Author
Contributor

I think it won't work. It's difficult to tune the CSS styles mixing with a Vue app element. So we just put the Vue app element inside a <div> we have full control.

I think it won't work. It's difficult to tune the CSS styles mixing with a Vue app element. So we just put the Vue app element inside a `<div>` we have full control.
wxiaoguang reviewed 2021-10-14 01:20:46 +00:00
templates/repo/view_file.tmpl
wxiaoguang commented 2021-10-14 01:20:46 +00:00
Author
Contributor

No caller to it, the HTML id #delete-message and #delete-file-form also disappeared.

Now the deletion function is provided by /org/repo/_delete/master/FILENAME.

No caller to it, the HTML id `#delete-message` and `#delete-file-form` also disappeared. Now the deletion function is provided by `/org/repo/_delete/master/FILENAME`.
wxiaoguang reviewed 2021-10-14 01:23:49 +00:00
templates/user/dashboard/repolist.tmpl
@ -1,8 +1,7 @@
<div id="app" class="six wide column">
<div id="dashboard-repo-list" class="six wide column">
<repo-search
wxiaoguang commented 2021-10-14 01:23:49 +00:00
Author
Contributor

Why not, it is used by DashboardRepoList.js

Why not, it is used by `DashboardRepoList.js`
wxiaoguang reviewed 2021-10-14 01:24:36 +00:00
web_src/js/components/ActivityHeatmap.vue
@ -70,4 +70,3 @@ export default {
},
};
</script>
<style scoped/>
wxiaoguang commented 2021-10-14 01:24:36 +00:00
Author
Contributor

This is an incorrect syntax. No effect.

This is an incorrect syntax. No effect.
wxiaoguang reviewed 2021-10-14 01:26:28 +00:00
web_src/js/components/DashboardRepoList.js
@ -0,0 +26,4 @@
},
organizations: {
type: Array,
default: () => [],
wxiaoguang commented 2021-10-14 01:26:28 +00:00
Author
Contributor

I just copied(moved) old code here, I won't make big change for the first commit, otherwise it will be difficult for us to view commit history and diff. Any refactor should happen in future after the move.

I just copied(moved) old code here, I won't make big change for the first commit, otherwise it will be difficult for us to view commit history and diff. Any refactor should happen in future after the move.
wxiaoguang reviewed 2021-10-14 01:26:54 +00:00
web_src/js/components/DashboardRepoList.js
@ -0,0 +30,4 @@
},
isOrganization: {
type: Boolean,
default: true
wxiaoguang commented 2021-10-14 01:26:54 +00:00
Author
Contributor

As above, no big change to first copy-move refactor.

As above, no big change to first copy-move refactor.
wxiaoguang reviewed 2021-10-14 01:27:07 +00:00
web_src/js/components/DashboardRepoList.js
@ -0,0 +252,4 @@
break;
default:
this.archivedFilter = 'unarchived';
break;
wxiaoguang commented 2021-10-14 01:27:06 +00:00
Author
Contributor

As above, no big change to first copy-move refactor.

As above, no big change to first copy-move refactor.
wxiaoguang reviewed 2021-10-14 01:27:12 +00:00
web_src/js/components/DashboardRepoList.js
@ -0,0 +274,4 @@
break;
default:
this.privateFilter = 'both';
break;
wxiaoguang commented 2021-10-14 01:27:12 +00:00
Author
Contributor

As above, no big change to first copy-move refactor.

As above, no big change to first copy-move refactor.
wxiaoguang reviewed 2021-10-14 01:27:17 +00:00
web_src/js/components/DashboardRepoList.js
@ -0,0 +291,4 @@
}
if (this.page < 1) {
this.page = 1;
}
wxiaoguang commented 2021-10-14 01:27:17 +00:00
Author
Contributor

As above, no big change to first copy-move refactor.

As above, no big change to first copy-move refactor.
wxiaoguang reviewed 2021-10-14 01:29:02 +00:00
web_src/js/components/VueComponentLoader.js
@ -0,0 +6,4 @@
let vueEnvInited = false;
function initVueEnv() {
if (vueEnvInited) return;
vueEnvInited = true;
wxiaoguang commented 2021-10-14 01:29:02 +00:00
Author
Contributor

inited is OK for abbr.

https://www.definify.com/word/inited

`inited` is OK for abbr. https://www.definify.com/word/inited
wxiaoguang reviewed 2021-10-14 01:29:09 +00:00
web_src/js/components/VueComponentLoader.js
@ -0,0 +16,4 @@
let vueSvgInited = false;
function initVueSvg() {
if (vueSvgInited) return;
vueSvgInited = true;
wxiaoguang commented 2021-10-14 01:29:08 +00:00
Author
Contributor

As above

As above
delvh reviewed 2021-10-14 09:30:19 +00:00
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +27,4 @@
## General Guidelines
We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
delvh commented 2021-10-14 09:30:19 +00:00
Contributor

That only applies inside the book title, not when referencing the book:
What that StackExchange discussion is saying is:
It is perfectly fine to call the book Lord of the rings.
You should, however, append the the when referencing the Lord of the rings series.

That only applies **inside** the _book title_, not when **referencing** the book: What that StackExchange discussion is saying is: It is perfectly fine to call the book `Lord of the rings`. You should, however, append the `the` when referencing **the** `Lord of the rings` series.
delvh reviewed 2021-10-14 09:31:34 +00:00
docs/content/doc/developers/guidelines-frontend.md
delvh commented 2021-10-14 09:31:34 +00:00
Contributor

Not a new section, a subsection.
Simply for the visual difference.

Not a new section, a subsection. Simply for the visual difference.
delvh reviewed 2021-10-14 09:33:40 +00:00
docs/content/doc/developers/hacking-on-gitea.en-us.md
delvh commented 2021-10-14 09:33:40 +00:00
Contributor

Yes, but it is already mentioned a few lines (5 to 10) above.
That way, it simply is duplicated.

Yes, but it is already mentioned a few lines (5 to 10) above. That way, it simply is duplicated.
delvh reviewed 2021-10-14 09:35:45 +00:00
routers/web/user/home.go
@ -110,1 +110,4 @@
ctx.PageData["dashboardRepoList"] = map[string]interface{}{
"searchLimit": setting.UI.User.RepoPagingNum,
}
delvh commented 2021-10-14 09:35:44 +00:00
Contributor

What I mean: This setting was previously rendered on the server-side, if I see that correctly.
Now the same thing seems to be rendered on the frontend with a more complicated mechanism?
Correct me if I'm wrong.

What I mean: This setting was previously rendered on the server-side, if I see that correctly. Now the same thing seems to be rendered on the frontend with a more complicated mechanism? Correct me if I'm wrong.
delvh reviewed 2021-10-14 09:37:03 +00:00
web_src/js/components/ActivityHeatmap.vue
@ -70,4 +70,3 @@ export default {
},
};
</script>
<style scoped/>
delvh commented 2021-10-14 09:37:03 +00:00
Contributor

Also makes sense.

Also makes sense.
wxiaoguang reviewed 2021-10-14 09:39:07 +00:00
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +27,4 @@
## General Guidelines
We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
wxiaoguang commented 2021-10-14 09:39:07 +00:00
Author
Contributor

Do you read Harry Potter? or Do you read the Harry Potter?

That's similar to the guideline names. Do we like Google Guideline?

`Do you read Harry Potter?` or `Do you read the Harry Potter?` That's similar to the guideline names. `Do we like Google Guideline?`
wxiaoguang reviewed 2021-10-14 09:45:06 +00:00
routers/web/user/home.go
@ -110,1 +110,4 @@
ctx.PageData["dashboardRepoList"] = map[string]interface{}{
"searchLimit": setting.UI.User.RepoPagingNum,
}
wxiaoguang commented 2021-10-14 09:45:06 +00:00
Author
Contributor

I don't understand why it's more complicated.

With old mechanism, a tag also must be written into the template. And frontend must query the tag.

The new mechanism, we just construct a map and frontend just reads it, nothing else. If you mean to write ctx.PageData["SearcLimit"] = xxx, I am totally against it, it pollutes the namespace. More code doesn't mean more complicated.

I don't understand why it's more complicated. With old mechanism, a tag also must be written into the template. And frontend must query the tag. The new mechanism, we just construct a map and frontend just reads it, nothing else. If you mean to write `ctx.PageData["SearcLimit"] = xxx`, I am totally against it, it pollutes the namespace. More code doesn't mean more complicated.
delvh reviewed 2021-10-14 09:45:19 +00:00
templates/repo/activity.tmpl
delvh commented 2021-10-14 09:45:18 +00:00
Contributor

Isn't the <div> by default an invisible element where it makes no difference if it exists or not?
I think this approach should behave just the same.

Isn't the `<div>` by default an invisible element where it makes no difference if it exists or not? I think this approach should behave just the same.
wxiaoguang reviewed 2021-10-14 09:49:53 +00:00
templates/repo/activity.tmpl
wxiaoguang commented 2021-10-14 09:49:53 +00:00
Author
Contributor

It won't work.

My code

image

Your code

image
It won't work. # My code <img width="1200" alt="image" src="https://user-images.githubusercontent.com/2114189/137294054-6b616bb4-72f1-422c-b5eb-2fa3c4810097.png"> # Your code <img width="1181" alt="image" src="https://user-images.githubusercontent.com/2114189/137294093-68fd08e1-ac17-4691-bc25-ff3fa7a109ba.png">
delvh reviewed 2021-10-14 09:59:05 +00:00
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +27,4 @@
## General Guidelines
We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
delvh commented 2021-10-14 09:59:05 +00:00
Contributor

That's similar to the guideline names. Do we like Google Guideline?

I would say that's part of the problem: guideline/ style guide are normal English words.
While they are also inside the title, for me it makes more sense to use them as the words they are, not as parts of the title.
To me, Do we like Google Guideline? sounds like incorrect English.

> That's similar to the guideline names. `Do we like Google Guideline?` I would say that's part of the problem: `guideline`/ `style guide` are normal English words. While they are also inside the title, for me it makes more sense to use them as the words they are, not as parts of the title. To me, `Do we like Google Guideline?` sounds like incorrect English.
delvh reviewed 2021-10-14 10:00:16 +00:00
routers/web/user/home.go
@ -110,1 +110,4 @@
ctx.PageData["dashboardRepoList"] = map[string]interface{}{
"searchLimit": setting.UI.User.RepoPagingNum,
}
delvh commented 2021-10-14 10:00:16 +00:00
Contributor

That is also true.
Okay, then I'm fine with it.

That is also true. Okay, then I'm fine with it.
delvh reviewed 2021-10-14 10:00:44 +00:00
templates/repo/activity.tmpl
delvh commented 2021-10-14 10:00:44 +00:00
Contributor

Interesting. Then ignore my comment.

Interesting. Then ignore my comment.
wxiaoguang reviewed 2021-10-14 10:03:02 +00:00
templates/repo/activity.tmpl
wxiaoguang commented 2021-10-14 10:03:02 +00:00
Author
Contributor

That's how Vue works. If we want to add CSS style to the root element, I think it must be added to the template directly. Otherwise the attached element will be replaced.

That's how Vue works. If we want to add CSS style to the root element, I think it must be added to the template directly. Otherwise the attached element will be replaced.
6543 (Migrated from github.com) reviewed 2021-10-14 16:30:11 +00:00
docs/content/doc/developers/guidelines-frontend.md
@ -0,0 +27,4 @@
## General Guidelines
We recommend [Google HTML/CSS Style Guide](https://google.github.io/styleguide/htmlcssguide.html) and [Google JavaScript Style Guide](https://google.github.io/styleguide/jsguide.html)
6543 (Migrated from github.com) commented 2021-10-14 16:30:10 +00:00
Owner

this is bike shading ... and Germans do like the :D (I use it to much too - it's not so common if you listen to native speaker ....)

anyway lets just leave it as is!

this is bike shading ... and Germans do like the :D (I use it to much too - it's not so common if you listen to native speaker ....) anyway lets just leave it as is!
6543 (Migrated from github.com) reviewed 2021-10-14 16:33:16 +00:00
docs/content/doc/developers/hacking-on-gitea.en-us.md
6543 (Migrated from github.com) commented 2021-10-14 16:33:16 +00:00
Owner

it's for the beginners ... let it be duplicated

it's for the beginners ... let it be duplicated
6543 (Migrated from github.com) approved these changes 2021-10-14 16:49:38 +00:00
lunny approved these changes 2021-10-15 02:17:17 +00:00
zeripath reviewed 2021-10-18 19:57:18 +00:00
templates/base/head.tmpl
@ -36,3 +28,3 @@
csrf: '{{.CsrfToken}}',
PageData: {{ .PageData }},
pageData: {{ .PageData }},
HighlightJS: {{if .RequireHighlightJS}}true{{else}}false{{end}},
zeripath commented 2021-10-18 19:57:18 +00:00
Contributor

The dashboard repo list depended on the uid and removing this has broken the dashboard repo list.

The dashboard repo list depended on the uid and removing this has broken the dashboard repo list.
zeripath reviewed 2021-10-18 20:03:15 +00:00
templates/base/head.tmpl
@ -36,3 +28,3 @@
csrf: '{{.CsrfToken}}',
PageData: {{ .PageData }},
pageData: {{ .PageData }},
HighlightJS: {{if .RequireHighlightJS}}true{{else}}false{{end}},
zeripath commented 2021-10-18 20:03:14 +00:00
Contributor

context_uid was used to fix the dashboard repo list for organisations too.

context_uid was used to fix the dashboard repo list for organisations too.
delvh reviewed 2021-10-18 20:04:52 +00:00
templates/base/head.tmpl
@ -36,3 +28,3 @@
csrf: '{{.CsrfToken}}',
PageData: {{ .PageData }},
pageData: {{ .PageData }},
HighlightJS: {{if .RequireHighlightJS}}true{{else}}false{{end}},
delvh commented 2021-10-18 20:04:52 +00:00
Contributor

But there has to be a better way to achieve this than adding it as a global header for every single page.
I dunno, what about the pageData mechanism?

But there has to be a better way to achieve this than adding it as a **global header for every single page**. I dunno, what about the `pageData` mechanism?
delvh reviewed 2021-10-18 20:06:59 +00:00
templates/base/head.tmpl
@ -36,3 +28,3 @@
csrf: '{{.CsrfToken}}',
PageData: {{ .PageData }},
pageData: {{ .PageData }},
HighlightJS: {{if .RequireHighlightJS}}true{{else}}false{{end}},
delvh commented 2021-10-18 20:06:59 +00:00
Contributor

Also, in what way did it break the dashboard repo list? Seemed fine to me, and I have a PR currently where I refactor that file (#17340)…

Also, in what way did it break the `dashboard repo list`? Seemed fine to me, and I have a PR currently where I refactor that file (#17340)…
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
lunny
6543
Labels
Clear labels   
backport/done

   
backport/v1.0

   
backport/v1.1

   
backport/v1.10

   
backport/v1.11

   
backport/v1.12

   
backport/v1.13

   
backport/v1.14

   
backport/v1.15

   
backport/v1.2

   
backport/v1.3

   
backport/v1.4

   
backport/v1.5

   
backport/v1.6

   
backport/v1.7

   
backport/v1.8

   
backport/v1.9

   
bounty

   
changelog

   
dependencies

Pull requests that update a dependency file

  
frontport/done

   
frontport/main

   
good first issue

Likely to be an easy fix

  
Hacktoberfest

   
hacktoberfest-accepted

   
in progress

   
kind/api

   
kind/breaking

   
kind/bug

   
kind/build

   
kind/deployment

   
kind/deprecated

   
kind/docs

   
kind/enhancement

   
kind/feature

   
kind/lint

   
kind/misc

   
kind/moderation

features for moderateing spam, abuse, ...

  
kind/package

   
kind/proposal

   
kind/question

   
kind/refactor

   
kind/regression

   
kind/security

   
kind/summary

   
kind/testing

   
kind/translation

   
kind/ui

   
kind/upstream-related

An issue that is caused by an upstream dependency

  
kind/usability

   
kind/ux

   
lgtm/done

   
lgtm/need 1

   
lgtm/need 2

   
performance/bigrepo

Performance Issues affecting Big Repositories

  
performance/cpu

   
performance/memory

Performance issues affecting memory use

  
performance/speed

performance issues with slow downs

  
priority/critical

   
priority/low

   
priority/maybe

   
priority/medium

   
proposal/rejected

   
reviewed/confirmed

Issue has been reviewed and confirmed to be present or accepted to be implemented

  
reviewed/duplicate

   
reviewed/fixed

A fix for this issue is already in the attached milestone

  
reviewed/invalid

   
reviewed/not-a-bug

The reported issue is the intended behavior

  
reviewed/wontfix

   
skip-changelog

   
stale

   
status/blocked

   
status/needs-feedback

   
status/wip

   
theme/2fa

   
theme/authentication

   
theme/avatar

   
theme/backup-restore

   
theme/docker

Docker themed issued

  
theme/federation

   
theme/issues

   
theme/kanban

   
theme/markdown

   
theme/migration

Migration issues

  
theme/mobile

   
theme/pr

   
theme/signing

   
theme/sqlite

   
theme/timetracker

   
theme/webhook

   
theme/wiki
No Label
backport/done
backport/v1.0
backport/v1.1
backport/v1.10
backport/v1.11
backport/v1.12
backport/v1.13
backport/v1.14
backport/v1.15
backport/v1.2
backport/v1.3
backport/v1.4
backport/v1.5
backport/v1.6
backport/v1.7
backport/v1.8
backport/v1.9
bounty
changelog
dependencies
frontport/done
frontport/main
good first issue
Hacktoberfest
hacktoberfest-accepted
in progress
kind/api
kind/breaking
kind/bug
kind/build
kind/deployment
kind/deprecated
kind/docs
kind/enhancement
kind/feature
kind/lint
kind/misc
kind/moderation
kind/package
kind/proposal
kind/question
kind/refactor
kind/regression
kind/security
kind/summary
kind/testing
kind/translation
kind/ui
kind/upstream-related
kind/usability
kind/ux
lgtm/done
lgtm/need 1
lgtm/need 2
performance/bigrepo
performance/cpu
performance/memory
performance/speed
priority/critical
priority/low
priority/maybe
priority/medium
proposal/rejected
reviewed/confirmed
reviewed/duplicate
reviewed/fixed
reviewed/invalid
reviewed/not-a-bug
reviewed/wontfix
skip-changelog
stale
status/blocked
status/needs-feedback
status/wip
theme/2fa
theme/authentication
theme/avatar
theme/backup-restore
theme/docker
theme/federation
theme/issues
theme/kanban
theme/markdown
theme/migration
theme/mobile
theme/pr
theme/signing
theme/sqlite
theme/timetracker
theme/webhook
theme/wiki
Milestone
Clear milestone
No items
No Milestone
1.16.0
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: lunny/gitea#17301
No description provided.
Delete Branch "frontend-refactor"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?