-
Notifications
You must be signed in to change notification settings - Fork 76
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
fix(academy): typos, updates and clarifications #1218
Conversation
MatousMarik
commented
Sep 17, 2024
- fix typos, mainly excess**
- update google accept cookies
- update google search element selection
- logical correction
typo TypeScript will yell when 'images' is not present - removeImages: true
…es: true. Merge remote-tracking branch 'origin/patch-2' into fix/academy
…ent session language (e.g. "Hledat") and also works just fine
since it should handle every single error it should fire on each error -> error handler, not only on the last one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing, thanks a ton
@MatousMarik Some checks are failing under the Pull Request. Would you prefer me to go through them and fix them, then merging your work if all is okay, or would you rather go and dive into them yourself? |
@@ -36,15 +36,15 @@ Let's first focus on the first 3 steps listed above. By using `page.click()` and | |||
<TabItem value="Playwright" label="Playwright"> | |||
|
|||
```js | |||
// Click the "I agree" button | |||
// Click the "Accept all" button |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also address #418 when we're already at this 💡
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, but it is fine since the reader should reflect on it in academy tutorials. This fix addresses the consistency between the text and the examples.
@honzajavorek I fixed the typos and capitalization errors, but I would be glad if you could do the rest (missing some auth token). |
I will take a look then! |
The token issues happen because the PR is from your personal fork of the repo. The current setup, which is in place just a few months, doesn't allow external contributors to run the checks. It's a known issue, but low priority. I'm not sure if you have full access to the docs repo, but I do, so I'll create new PR with your branch. |
I created #1236, but it magically triggered also checks under this PR 🪄 Still unsure if this is expected GitHub's behavior or a bug, but it's green now and you've got approving review from @metalwarrior665, so I see no reason not to merge this right away. |