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

feat(PN-12551): generate html template script #13

Open
wants to merge 64 commits into
base: feature/bilinguismo-amministrativo
Choose a base branch
from

Conversation

SarahDonvito
Copy link
Contributor

@SarahDonvito SarahDonvito commented Sep 9, 2024

To test

  • delete output/templates/*
  • run yarn generate to check template are correctly produced

package.json Outdated
"description": "This repo contains the mail templates used by Piattaforma Notifiche, created using [MJML](https://mjml.io/) markup language.",
"main": "index.js",
"scripts": {
"test": "echo \"Error: no test specified\" && exit 1"
Copy link
Contributor

Choose a reason for hiding this comment

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

@SarahDonvito ti suggerisco qui di aggiungere uno script tipo "generate" che chiama lo script node generateHtmlTemplate.
da qui possiamo lavorare sui parametri da passare in input allo script node per la generazione dei template

package.json Outdated Show resolved Hide resolved
}

async function generateHtmlTemplates() {
const languages = ["de"]; // Elenca le lingue supportate
Copy link
Contributor

Choose a reason for hiding this comment

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

qui avremo "de", "fr", "sl"

Copy link
Contributor

Choose a reason for hiding this comment

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

dobbiamo aggiungere anche it, corretto? @SarahDonvito la generazione del template base potrebbe comunque passare da qui, ti torna?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

non so se avrebbe senso dal momento che vorrebbe dire realizzare a mano l'html ita qui e copiarlo in delivery-push

Copy link
Contributor

Choose a reason for hiding this comment

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

così avremo la separazione del contenuto dal layout però (soprattutto in ottica di template engine)

@SarahDonvito SarahDonvito changed the base branch from develop to feature/bilinguismo-amministrativo October 17, 2024 10:02
package.json Outdated
"keywords": [],
"author": "",
"license": "SEE LICENSE IN LICENSE",
"dependencies": {
Copy link
Contributor

Choose a reason for hiding this comment

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

forse è meglio settarle come dev-dependencies

@SarahDonvito SarahDonvito marked this pull request as ready for review November 7, 2024 11:57
@AndreaCimini90 AndreaCimini90 self-assigned this Nov 13, 2024
.idea/.gitignore Outdated
@@ -0,0 +1,3 @@
# Default ignored files
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this file?

.idea/vcs.xml Outdated
@@ -0,0 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need this file?

"/i18n",
`${language}.json`
);
const result = await fs.readJson(filePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

when using await, the code must be surrounded by a try catch block. insert try catch blocks in all cases and log the errors with a custom message.
For example console.error("Error during translations loading", e)

// generazione template BILINGUA
async function generateHtmlTemplates() {
const languages = ["it", "de", "fr", "sl"]; // Elenca le lingue supportate
const templates = [
Copy link
Contributor

Choose a reason for hiding this comment

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

I should avoid to define the list in this way because, if we add a new template, we have to remember to edit this file too.
Get the templates reading the content of the resources/templates directory

const templateLang =
language === "it"
? templateContent.replaceAll(
/<[^>]*data-hide-on-it="true"[^>]*>[\s\S]*?<\/[^>]*>/g,
Copy link
Contributor

Choose a reason for hiding this comment

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

This not work on nested elements. Try on regex101 and use and example string like this

<div>
 <div-to-remove data-hide-on-it="true">
  <p>
   This a text for testing
  </p>
 </div-to-remove>
</div>

you will see that the regexp will stop at p closing tag. This happen because the regular expression stops to the first closing tag regardless of whether it is the same as opening with data-hide-on-it.

Maybe we can manage the task in a simple way:

  • or define a custom tag (i.e. <start-lang></end-lang>). this tag must be always removed with the difference that in italian must me removed also the content
  • or define a comment (i.e. <!-- START lang --> and <!-- END lang -->)

@AndreaCimini90 AndreaCimini90 self-requested a review November 15, 2024 16:22
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.

5 participants