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

Frontend Plan #17149

Closed
wxiaoguang opened this issue Sep 25, 2021 · 16 comments
Closed

Frontend Plan #17149

wxiaoguang opened this issue Sep 25, 2021 · 16 comments
Labels
topic/ui-interaction Change the process how users use Gitea instead of the visual appearance type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Sep 25, 2021

Background

The frontend needs a big refactor now.

  1. web_src/index.js is too big to add new code (or modify/debug).
  2. code doesn't do what it should do, eg: initVueApp/initVueComponents is quite misleading
  3. Vue/fomantic/jQuery are mixed together, and we may need JSX Enable JSX support in frontend #15293
  4. no easy way to pass complex data structure from backend to frontend. eg: code like var ActivityTopAuthors = {{Json .ActivityTopAuthors | SafeJS}}; is not a good idea.
  5. difficult to shared data between modules, eg: how can different frontend modules get current issue index?
    <!-- I know, there is probably a better way to do this -->
  6. the dependency and coupling are not clear, many frontend code work like if ( $('.xxx').length == 0) return; , we do not know what pages need this function, and these css class names may conflict. A little change may break something.
  7. many events are attached to document, they may conflict. Add namespace to global click event #16833
  8. some problems should be resolved by framework: Prevent double click new issue/pull/comment button #16157 , Click buttons quickly in some forms will submit repeated/duplicated requests #17111

Guideline

  1. Every feature should be put in separated files/directories.
  2. HTML id/css-class-name should use kebab-case.
  3. HTML id/css-class-name used by JavaScript top-level selector should be unique in whole project, and should contain feature related keywords.
  4. jQuery events across different features should use their own namespaces.

Plan

What frontend frameworks should be used?

  • fomantic will stay as long as possible. And MVC is SEO friendly.
  • jQuery is a part of fomantic.
  • Vue is already used, so we can continue to use it, but we should upgrade it to Vue3 and do a refactor. Vue3 is handy to implement complex UI.
  • JSX is nice to have and it works well with Vue3. But we do not choose it now (according to the discussions)
  • ReactJS or other MVVM? I think it doesn't make big difference from Vue3 in this project, however if we drop Vue then we must migrate many components, not worthy.

I prefer fomantic + Vue3

How to pass backend data to frontend?

  • For simple data, it can be put into HTML data-xxx tag during template rendering

  • For complex data like array/map, it can be put into window.config.pageData.xxx. We write some common code to render pageData into JSON automatically in base template:

<script>
window.config.pageData = {{ .PageData }}
</script>

Then backend work will be easy:

ctx.PageData["xxx"] = someMap

How to init frontend functions in a page?

We have two choices (I prefer keep and improve current mechanism)

Frontend functions depned on templates

It is the mechanism what we are using now. In index.js it calls every init function, every init function checks whehter current page has some DOM elements. If current page has the matched DOM elements, then the feature will be loaded.

Pros:

  • Simple, it doesn't change much with current mechanism

Cons:

  • init functions can not have arguments
  • There will be tens of init functions calls in index.js (maybe more than a hundred soon?)

Templates call frontend init functions

index.js exports init functions. Templates render scripts in HTML like this:

<script>
window.config.ready(function($gitea)) {
    $gitea.initIssueList();
    $gitea.initUserList(extraData);
}
</script>

When the page is loaded, index.js calls every function registered by window.config.ready.

Pros:

  • It saves init time because only necessary functions are called.
  • Flexible, template code can do more things when JavaScript modules are ready.

Cons:

  • Order/dependency problem?
  • Some init functions can only be called once while others are called many times?

Other suggestions?

TODO

Refactor Steps

  1. (DONE) Complete a formal frontend guideline document in docs/content/doc/developers/frontend.md. The guideline can come from this draft.

  2. (DONE) Split web_src/index.js into small files in web_src/features/. Add function calls to initXxx in index.js as before. This step can be done ASAP, and must be reviewed and approved quickly to prevent from further conflicts. Any new feature should be placed in web_src/features/ later.

  3. (DONE) Introduce a common mechanism to pass backend data to frontend. eg: window.config.pageData.

  4. (keep current mechanism) Introduce or improve a common mechanism to call page related init functions.

  5. [FUTURE] Upgrade Vue from 2 to 3.

  6. [DONE] If a change is made to legacy frontend code, the author should also follow the guideline to refactor related modules.

  7. [FUTURE] Implement common and reusable code to resolve legacy frontend issues.

@wxiaoguang wxiaoguang added type/proposal The new feature has not been accepted yet but needs to be discussed first. pr/wip This PR is not ready for review frontport/main This PR is not targeting the main branch. Still, its changes should also be added there labels Sep 25, 2021
@wxiaoguang
Copy link
Contributor Author

The draft is updated. Please help to improve.

@techknowlogick techknowlogick added topic/ui Change the appearance of the Gitea UI kind/usability topic/ui-interaction Change the process how users use Gitea instead of the visual appearance and removed frontport/main This PR is not targeting the main branch. Still, its changes should also be added there labels Sep 25, 2021
@lafriks
Copy link
Member

lafriks commented Sep 25, 2021

We should definitely move to vue3 and move away from jquery by rewriting parts as vue components but I hardly disagree on moving to JSX

@axifive
Copy link
Member

axifive commented Sep 25, 2021

Another way: save server-templates front(maybe later simplify to base functionality) and in parallel start to rewrite new with vue and api

@wxiaoguang
Copy link
Contributor Author

@lafriks

About JSX:

Pros

  • Simple syntax, just like JavaScript back-tick string (template literals), there is no different feeling when reading template literals or JSX.
  • Safe, no worry about XSS in most cases
  • Efficiency, the HTML structure is generated by compiler, browsers do not need to parse the HTML string to DOM (which is slow)
  • Main trend, like ReactJS, Preact, Mithril, Vue3, etc, they all like JSX
  • Reusable code can be written inside a Vue componment (without JSX, it's difficult to write multiple components in a single .vue file)
  • If we want to move away from jQuery, we must have a simple method to construct HTML DOM, JSX is a good choice.

Cons

  • Introduce a new syntax and a new compiler plugin

General speaking, I can see there is a lot of benefits when using JSX. If less a half of members disagree JSX, then we can continue?

@axifive

About server-templates and Vue and API

In my mind, this frontend refactor should be done in one or two weeks, so we just make things clear first.

I can not imagine that the frontend can be switched to Vue + API in short time, the frontend is big. It's OK for me to continue to use MVC templates (I can accept everything as long as it is clear and easy to maintain), so the MVC template is not a major problem now, and it is easy to debug and SEO-friendly. We can leave the rewriting work to each module, and choose the best solution one by one then.

@lunny
Copy link
Member

lunny commented Sep 26, 2021

@lafriks vue3 also supports JSX, so I don't think JSX is totally unacceptable. I think we can mix template or JSX according the situations.

@lafriks
Copy link
Member

lafriks commented Sep 26, 2021

@wxiaoguang

Simple syntax, just like JavaScript back-tick string (template literals), there is no different feeling when reading template literals or JSX.

Not true, as it's where hard to guess what's final HTML will look like especially for large components or pages, it's also hard to debug them. And in my opinion it looks very weird that JS is mixed with HTML without any separation.

Safe, no worry about XSS in most cases

Same with Vue templates

Efficiency, the HTML structure is generated by compiler, browsers do not need to parse the HTML string to DOM (which is slow)

Same with Vue templates that are compiled to VDomNodes

Reusable code can be written inside a Vue componment (without JSX, it's difficult to write multiple components in a single .vue file)

Not true, you can write functional Vue components just as import any other utility code inside .vue components

If we want to move away from jQuery, we must have a simple method to construct HTML DOM, JSX is a good choice.

We already have - .vue files 😉

As I already have said previously JSX looks like old shity PHP code when HTML was mixed all over the place with PHP.

I have worked on large react apps where all frontend is in JSX and I know what pain in the as* it can be... 😆

There are things also in Vue that are pain to work with like vuex etc but JSX won't help with that either 😃

@lafriks
Copy link
Member

lafriks commented Sep 26, 2021

I also don't want to seem against other parts of proposal, I fully agree that we need to make an order in fronted part, it's just that one tiny thing (jsx) I don't agree with :)

@techknowlogick
Copy link
Member

A note that we will also need to pay close attention to accessibility when updating the front end.

Unrelated to the above, a casual suggestion for using tailwindcss in changes (I think v3 is out soon).

@wxiaoguang
Copy link
Contributor Author

@lafriks OK, I just thought JSX is nice to have (ps: I have most experiences in PHP among all languages, a good guideline makes PHP code as strict as other modern languages, JSX is similar). JSX is not a must, so we just use fomantic + Vue3 then.

@techknowlogick there are so many codes depending on fomantic, it's not easy to switch to tailwindcss soon, maybe we can finish the first stage refactor soon (without introducing new frameworks), then discuss new frameworks later.

@lunny
Copy link
Member

lunny commented Sep 27, 2021

About tailwind, we can have a try on some page and then make decision if it should be continued.

@techknowlogick
Copy link
Member

then discuss new frameworks later.

sounds good!

@wxiaoguang
Copy link
Contributor Author

The PR #16770 uses the window.config.PageData mechanism for passing variables from backend to frontend.

If this plan is generally acceptable, I hope the PR can be approved and merged soon, then we can move on.

@delvh
Copy link
Member

delvh commented Sep 28, 2021

I have only one small nit about the PageData mechanism:
As far as I know and have seen in the JS files, camelCase is used to name variables.
So, shouldn't the variable then be called window.config.pageData instead of window.config.PageData?

Apart from that, I absolutely agree that such a mechanism is useful and needed.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Sep 28, 2021

@delvh about PageData: already discussed here: #16770 (review)

I think PageData is better because:

  1. Most other variables around look like this
  2. It indicates that this is not an ordinary variable for local or ad-hoc usage, it has special meaning and should not be modified manually.

======

Update: now we use pageData as advice

@lafriks
Copy link
Member

lafriks commented Sep 28, 2021

Still we should refactor this so that all variables are named camelCase as per JS naming conventions

@wxiaoguang
Copy link
Contributor Author

This issue has fulfilled its mission.

For frontend guidelines: https://docs.gitea.io/en-us/guidelines-frontend/
For other future works: new issue and new PR.

So we can close this one.

@wxiaoguang wxiaoguang removed the pr/wip This PR is not ready for review label Oct 17, 2021
@go-gitea go-gitea locked and limited conversation to collaborators Apr 28, 2022
@delvh delvh removed topic/ui Change the appearance of the Gitea UI kind/usability labels Oct 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/ui-interaction Change the process how users use Gitea instead of the visual appearance type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

6 participants