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

Create Progressive Web App #4730

Merged
merged 14 commits into from
Nov 27, 2018
Merged

Conversation

SohnyBohny
Copy link
Contributor

@SohnyBohny SohnyBohny commented Aug 16, 2018

Create a PWA by adding a app manifest and a service worker
This will allow users (especially on Android) to add the gitea website to the home-screen and use it like a native app.

Being served over https a service worker will cache all static content which will cause faster page loading and a better user experience.


TODOs:

  • Add 192x192 and 512x512 icons to the manifest.json
  • Complete list of static content in serviceworker.js
  • (Reorder head.tmpl)

@codecov-io
Copy link

codecov-io commented Aug 16, 2018

Codecov Report

Merging #4730 into master will decrease coverage by <.01%.
The diff coverage is 60%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4730      +/-   ##
==========================================
- Coverage   37.46%   37.46%   -0.01%     
==========================================
  Files         312      312              
  Lines       46507    46522      +15     
==========================================
+ Hits        17423    17428       +5     
- Misses      26595    26601       +6     
- Partials     2489     2493       +4
Impacted Files Coverage Δ
routers/routes/routes.go 84.69% <0%> (-0.78%) ⬇️
modules/templates/dynamic.go 62.66% <100%> (+5.09%) ⬆️
models/repo_indexer.go 44.49% <0%> (-3.82%) ⬇️
models/repo_list.go 64.55% <0%> (+1.26%) ⬆️
modules/process/manager.go 81.15% <0%> (+4.34%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e09fe48...98ad135. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 16, 2018
@lafriks
Copy link
Member

lafriks commented Aug 16, 2018

Can this be somehow changed not to have duplicate js files? Also if gitea is deployed with suburl this will not work

@SohnyBohny
Copy link
Contributor Author

Cuplicate js files? Where?

What do you mean with suburl? Subdomain should work. But a sub path might cause problems...
The files (manifest and service worker) should be generated dynamically (like html templates) to fix this. But to do that I would need somebody's help.

@lafriks
Copy link
Member

lafriks commented Aug 16, 2018

You coud add data-cache="true" to script tags for javascipt etc files that need to be cached and use javascript to select them:

var scripts = document.querySelectorAll('script[data-cache="true"]');
// loop over scripts and get src attribute

Just an idea, idk if that would work :)

@SohnyBohny
Copy link
Contributor Author

I don't think that this would work because service worker doesn't have access to the document.

@SohnyBohny
Copy link
Contributor Author

The solution might be to generate the service worker file dynamically like html templates. Also the mainfest has a namespace which should be generated dynamically.

@techknowlogick
Copy link
Member

@SohnyBohny An example of how to use a template to achieve what you are looking for is #3572

@SohnyBohny
Copy link
Contributor Author

Okay thank you - I will check this out when I have time (in ≈2 weeks)

@0rzech
Copy link
Contributor

0rzech commented Aug 23, 2018

What do you mean with suburl? Subdomain should work. But a sub path might cause problems...

@SohnyBohny AppSubURL variable stores URL path to Gitea instance [source]. In templates its name is AppSubUrl [source].

@lunny lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 28, 2018
@lunny lunny added this to the 1.x.x milestone Aug 28, 2018
@lunny
Copy link
Member

lunny commented Aug 28, 2018

CI failed

@SohnyBohny
Copy link
Contributor Author

What does this error mean?
Makefile:152: recipe for target 'fmt-check' failed

@lunny
Copy link
Member

lunny commented Aug 28, 2018

your golang code is not the same as the format program did.

@SohnyBohny
Copy link
Contributor Author

Finally CI build has passed :-)

@techknowlogick
Copy link
Member

@SohnyBohny one minor nit, but everything else looks great. Could TODO items be for another PR, or are they needed right away?

@SohnyBohny
Copy link
Contributor Author

The 192x192 and 512x512 icons are needed that chrome automatically shows an install banner on Android. Without them the User will not find out this feature.
I'm not sure how these icon are added. I have found an generate-images section in the Makefile but I'm not experienced with Makefiles at all.

@lunny
Copy link
Member

lunny commented Oct 29, 2018

But cannot work on chrome macOS? I have enabled PWA desktop but it seems it doesn't work.

@SohnyBohny
Copy link
Contributor Author

@lunny any error message? I think 512px sized icon is needed...

@lafriks lafriks modified the milestones: 1.x.x, 1.7.0 Oct 30, 2018
@lunny
Copy link
Member

lunny commented Nov 19, 2018

[Macaron] 2018-11-19 09:28:21: Started GET /css/theme-gitea.css for [::1]
2018/11/19 09:28:21 [D] Session ID: 529e98b5f23de857
2018/11/19 09:28:21 [D] CSRF Token: MWPHepbatiwdXEvp0ukobEJxZTU6MTU0MjU5MDg2OTM1NTAwNjAwMA==
2018/11/19 09:28:21 [I] [SQL] SELECT  TOP 1 "id", "lower_name", "name", "full_name", "email", "keep_email_private", "passwd", "must_change_password", "login_type", "login_source", "login_name", "type", "location", "website", "rands", "salt", "language", "created_unix", "updated_unix", "last_login_unix", "last_repo_visibility", "max_repo_creation", "is_active", "is_admin", "allow_git_hook", "allow_import_local", "allow_create_organization", "prohibit_login", "avatar", "avatar_email", "use_custom_avatar", "num_followers", "num_following", "num_stars", "num_repos", "description", "num_teams", "num_members", "diff_view_style" FROM "user" WHERE "lower_name"=? []interface {}{"css"}
2018/11/19 09:28:21 [D] Template: status/404
[Macaron] 2018-11-19 09:28:21: Completed GET /css/theme-gitea.css 404 Not Found in 100.550709ms

@lunny
Copy link
Member

lunny commented Nov 26, 2018

Another two TODOs?

@techknowlogick
Copy link
Member

@lunny I think last TODO can wait for another PR.

@bkcsoft bkcsoft 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 Nov 26, 2018
@SohnyBohny
Copy link
Contributor Author

Great @techknowlogick thank you

Of course the last TODO can wait (if it is neccessary at all). I was just not sure how you like the head of the html to be ordered.

For me it's done now. Of course the list should be up to date at any time.

Somewere in the docs or in the release notes there should be a note that the service worker has to be served over https (or localhost)

@bkcsoft bkcsoft 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 Nov 27, 2018
@techknowlogick techknowlogick merged commit 2949043 into go-gitea:master Nov 27, 2018
@techknowlogick techknowlogick added the type/changelog Adds the changelog for a new Gitea version label Nov 27, 2018
@lafriks
Copy link
Member

lafriks commented Nov 27, 2018

I think this list is already outdated :) We should really think of some kind of automated sync for it

@lunny
Copy link
Member

lunny commented Nov 28, 2018

Which list?

@lafriks
Copy link
Member

lafriks commented Nov 28, 2018

I think at least recently merged vue-calendar-heatmap, moment

@SohnyBohny
Copy link
Contributor Author

Workbox is exactly our usecase. I'll look into that as soon as possible and will create a new PR then.

@SohnyBohny SohnyBohny deleted the create_pwa branch December 15, 2018 16:14
@SohnyBohny
Copy link
Contributor Author


Is this the right place to keep discussing about this topic or should I open an issue for that?


Anyway:
I've looked into Workbox now. The way it works is by looking for specific file types in specified folders and automatically add them to the caching list while building time.

Most cacheable files are located under /vendor/plugins/ but they are not accessible in source folder. Could somebody please explain how this works or link me to a wiki page?

More specific I need following questions answered:

- What is the root of your web app (i.e. which directory do you deploy)?
- Which file types would you like to precache? (Located in the folder specified above or in subfolders)

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
@delvh delvh removed the type/changelog Adds the changelog for a new Gitea version label Oct 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants