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.

Sign up for GitHub

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

Jump to bottom

Scripts: async, defer #229

Merged
merged 11 commits into from
Mar 17, 2024
Merged

Scripts: async, defer #229

merged 11 commits into from
Mar 17, 2024

Conversation

larissaiurk
Copy link
Contributor

No description provided.

@javascript-translate-bot javascript-translate-bot requested a review from a team October 23, 2021 03:08
@CLAassistant
Copy link

CLAassistant commented Oct 23, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@javascript-translate-bot javascript-translate-bot mentioned this pull request Oct 23, 2021
Open
@odsantos
Copy link
Contributor

Hi @larissaiurk,

Still available for this task?
There is a conflict here.

Please, let me know if you have not time, and I might be able to resolve it.

Thanks,
Osvaldo.

@nazarepiedady
Copy link
Member

@Peruibeloko, @jonnathan-ls, do you have time to see what is possible to do in relation to this pull request?

Peruibeloko
Peruibeloko reviewed Mar 6, 2024
View reviewed changes
2-ui/5-loading/02-script-async-defer/article.md Outdated
1. Scripts can't see DOM elements below them, so they can't add handlers etc.
2. If there's a bulky script at the top of the page, it "blocks the page". Users can't see the page content till it downloads and runs:
1. Os scripts não conseguem ver os elementos da DOM abaixo deles, então não conseguem manipular eventos, etc.
2. Se houver um script volumoso no topo da página, ele vai "bloqueia a página". Os usuários não podem ver o conteúdo da página até que ele faça o download e execute:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2. Se houver um script volumoso no topo da página, ele vai "bloqueia a página". Os usuários não podem ver o conteúdo da página até que ele faça o download e execute:
2. Se houver um script volumoso no topo da página, ele vai "bloquear a página". Os usuários não podem ver o conteúdo da página até que ele faça o download e execute:

Copy link
Contributor

Choose a reason for hiding this comment

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

Ajustado

@nazarepiedady
Copy link
Member

nazarepiedady commented Mar 6, 2024 •
edited
Loading

@Peruibeloko, could you help to fix the conflicts, update, and approve it?

jonnathan-ls
jonnathan-ls approved these changes Mar 17, 2024
View reviewed changes
Copy link
Contributor

@jonnathan-ls jonnathan-ls left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

I added simple adjustments (0dfdf25) and resolved the merge conflict (b0b6ae0)

@jonnathan-ls
Copy link
Contributor

jonnathan-ls commented Mar 17, 2024 •
edited
Loading

... could you help to fix the conflicts, update, and approve it?

During the review, I added simple adjustments (including the suggestion requested by @Peruibeloko) and also conflict resolution @nazarepiedady

/done

nazarepiedady
nazarepiedady requested changes Mar 17, 2024
View reviewed changes
Copy link
Member

@nazarepiedady nazarepiedady left a comment

Choose a reason for hiding this comment

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

It is good, but there are some little changes needed.

2-ui/5-loading/02-script-async-defer/article.md Outdated
Assim como `defer`, o atributo `async` é ignorado se a tag `<script>` não tiver `src`.
```

## Dynamic scripts
Copy link
Member

Choose a reason for hiding this comment

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

It was not translated yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

2-ui/5-loading/02-script-async-defer/article.md Outdated

## Dynamic scripts

There's one more important way of adding a script to the page.
Copy link
Member

Choose a reason for hiding this comment

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

It also was not translated yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

Done

@javascript-translate-bot
Copy link

Please make the requested changes. After it, add a comment "/done".
Then I'll ask for a new review 👻

@nazarepiedady
Copy link
Member

@jonnathan-ls, there are some changes that need to be translated.

@jonnathan-ls
Copy link
Contributor

@nazarepiedady Thanks for watching, pending translation added

/done

nazarepiedady
nazarepiedady approved these changes Mar 17, 2024
View reviewed changes
Copy link
Member

@nazarepiedady nazarepiedady left a comment

Choose a reason for hiding this comment

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

It is good to be applied.

@nazarepiedady nazarepiedady merged commit dd27063 into javascript-tutorial:master Mar 17, 2024
1 check passed
@javascript-translate-bot
Copy link

Thank you 💖 I updated the Progress Issue #1 🎉 🎉 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Peruibeloko Peruibeloko Peruibeloko left review comments

@jonnathan-ls jonnathan-ls jonnathan-ls approved these changes

@nazarepiedady nazarepiedady nazarepiedady approved these changes

@odsantos odsantos Awaiting requested review from odsantos

Assignees
No one assigned
Labels
needs +1
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@larissaiurk @CLAassistant @odsantos @nazarepiedady @jonnathan-ls @javascript-translate-bot @Peruibeloko