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

I18n server #61

Closed
wants to merge 6 commits into from
Closed

I18n server #61

wants to merge 6 commits into from

Conversation

gtico80
Copy link
Contributor

@gtico80 gtico80 commented May 13, 2019

Issue #45 labels main and infrastructure

@libremente libremente requested review from sebbalex and libremente and removed request for sebbalex May 13, 2019 12:26
@libremente
Copy link
Member

Thanks @gtico80 for the precious work! We will review it during the week and give you some feedback. 👍 @sebbalex

Copy link
Member

@sebbalex sebbalex left a comment

Choose a reason for hiding this comment

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

Actually we have four views in flow:

  • main-content
  • email-sent
  • register-confirm
  • confirmed

We must provide localisation for all views and even for email.mst which represent the template for email sent

</div>

<div class="form-group active child">
<input type="text" class="form-control-plaintext active readonly" id="nomeAmministrazione" tabindex="-1"
required>
<label for="nomeAmministrazione">Amministrazione *</label>
<label for="nomeAmministrazione">{{main.nomeAmministrazione}}</label>
Copy link
Member

Choose a reason for hiding this comment

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

it should be {{main.labelnomeAmministrazione}}

@libremente
Copy link
Member

Actually we have four views in flow:

* main-content

* email-sent

* register-confirm

* confirmed

We must provide localisation for all views and even for email.mst which represent the template for email sent

Quoting @sebbalex 's comment, I also believe that in order to merge this PR and close the relative issue an extra effort should be done to consolidate all the views. Are you able to proceed in this direction @gtico80 or you need some assistance? Thanks a lot for your precious work! 🚀

@libremente
Copy link
Member

Hi @gtico80, I believe that in order to be able to correctly review this PR it is necessary to rebase your branch against our master and consequently run the linter. When trying to run it (with node 12.4.0), I found several problems and the application could not even start. If you need assistance during the rebasing process please let us know! Thanks a lot / cc @sebbalex

@libremente
Copy link
Member

libremente commented Jul 25, 2019

Hi @gtico80, if you want we can rebase your branch against our master and push on your fork so that we can test this PR further and merge it. What do you think? Thanks a lot!

@gtico80
Copy link
Contributor Author

gtico80 commented Jul 26, 2019

We can try but probably it is better to fix issue #65 otherwise it is difficult to test it

@libremente
Copy link
Member

libremente commented Jul 26, 2019

@gtico80 you are right, we will try to work on that issue next week and we'll let you know. Thanks! cc @sebbalex

@bfabio
Copy link
Member

bfabio commented Oct 29, 2021

@gtico80 we are planning on refactoring the whole site and use a static approach for the frontend, I'm gonna close this one as it's stale but despite that, your work on it was appreciated.

@bfabio bfabio closed this Oct 29, 2021
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.

4 participants