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

Break everything and introduce i18n #69

Merged
merged 1 commit into from
Sep 19, 2024
Merged

Break everything and introduce i18n #69

merged 1 commit into from
Sep 19, 2024

Conversation

benoit74
Copy link
Collaborator

@benoit74 benoit74 commented Sep 13, 2024

Code work for #46 and #53 ; translation will then have to be setup on translatewiki

Rationale

  • Introduce i18n of the UI and the emails
  • Move ui to Vue 3, Vuetify, Pinia, Vue-Router
  • Move api to FastAPI, hatch
  • Fix many small glitches linked to recent Zimfarm changes
  • Enhance some messages
  • Upgrade to Node 20 and Python 3.12
  • Adapted CI to check code QA on both api and ui + add automated tests
  • Split API and UI for proper deployment (two Dockerfile, two packages, adapt CI, ... work on kiwix/operations will follow)

Nota:

  • for now PR contains fake and incomplete translations to fr and fa, but these will be dropped before merging, this is used only for testing purpose

@benoit74 benoit74 force-pushed the add_i18n_and_rewrite branch 9 times, most recently from 04a31a1 to 76e4d25 Compare September 13, 2024 11:49
Copy link

codecov bot commented Sep 13, 2024

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

@benoit74 benoit74 marked this pull request as ready for review September 13, 2024 12:10
@benoit74 benoit74 requested a review from rgaudin September 13, 2024 12:10
Copy link
Member

@rgaudin rgaudin left a comment

Choose a reason for hiding this comment

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

That's something! Thank you.

I tested it and it seems to work fine. I was somehow surprised that the UI looks the same 😀

On the python side, I think there are a couple of place with poor type hints (zimfarm module?).

On the Vue part, I mostly skimmed through the code but it looks good and coherent.

On personal taste, I find the user feedback too dim. If there is an error in making a request, theres just a light toast at the bottom of the page that disappears after 5s.
I had to make several requests to see it.

.github/workflows/Publish.yml Outdated Show resolved Hide resolved
.github/workflows/QA.yml Show resolved Hide resolved
.github/workflows/Tests.yml Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
.pre-commit-config.yaml Outdated Show resolved Hide resolved
api/src/zimitfrontend/routes/utils.py Outdated Show resolved Hide resolved
api/src/zimitfrontend/routes/utils.py Outdated Show resolved Hide resolved
api/src/zimitfrontend/templates/email_body.html Outdated Show resolved Hide resolved
api/src/zimitfrontend/utils.py Show resolved Hide resolved
ui/src/App.vue Show resolved Hide resolved
@benoit74
Copy link
Collaborator Author

I tested it and it seems to work fine. I was somehow surprised that the UI looks the same 😀

Mostly 🤫

On the python side, I think there are a couple of place with poor type hints (zimfarm module?).

Fixed, thank you

On the Vue part, I mostly skimmed through the code but it looks good and coherent.

OK, thank you

On personal taste, I find the user feedback too dim. If there is an error in making a request, theres just a light toast at the bottom of the page that disappears after 5s.

I think you refer to home page issue, when it fails to fetch offliner definition. This is indeed something I forgot about. I changed the toast to red + I added a message in red as well which stays forever. On the page about the task status, there is a red message when we fail to fetch the task.

@benoit74 benoit74 force-pushed the add_i18n_and_rewrite branch from 0117b97 to 5d66daf Compare September 17, 2024 14:50
- Introduce i18n of the UI and the emails
- Move ui to Vue 3, Vuetify, Pinia, Vue-Router
- Move api to FastAPI, hatch
- Fix many small glitches linked to recent Zimfarm changes
- Enhance some messages
@benoit74 benoit74 force-pushed the add_i18n_and_rewrite branch from 5d66daf to 648edb1 Compare September 19, 2024 09:28
@benoit74 benoit74 merged commit b061c16 into main Sep 19, 2024
7 checks passed
@benoit74 benoit74 deleted the add_i18n_and_rewrite branch September 19, 2024 09:32
@benoit74
Copy link
Collaborator Author

(merging because confirmed with rgaudin that further review is not necessary once conversations are taken care of)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants