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

Refactor the BrowserContext class docs for xk6-browser #723

Merged
merged 14 commits into from
Jul 27, 2022
Merged

Refactor the BrowserContext class docs for xk6-browser #723

merged 14 commits into from
Jul 27, 2022

Conversation

ankur22
Copy link
Contributor

@ankur22 ankur22 commented Jul 7, 2022 •
edited
Loading

Closes: grafana/xk6-browser#441

  • Double checked the wording
  • Tested features and linked to issues where necessary.

All features for the BrowserContext class were tested on:

  • Mac OS Monterey
  • Windows 11

@ankur22 ankur22 requested review from inancgumus and imiric July 7, 2022 17:04
@CLAassistant
Copy link

CLAassistant commented Jul 7, 2022 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@ankur22 ankur22 changed the base branch from main to docs/582-xk6-browser July 7, 2022 17:04
@ankur22 ankur22 changed the title Refactor the BrowserContext class docs for xk6-browser Draft: Refactor the BrowserContext class docs for xk6-browser Jul 8, 2022
@ankur22 ankur22 force-pushed the docs/582-xk6-browser branch from b4e7cf8 to b2e2387 Compare July 12, 2022 11:42
@ankur22 ankur22 force-pushed the docs/441-xk6-browser-browsercontext-class-docs branch 2 times, most recently from 218bb75 to f428e5b Compare July 13, 2022 15:48
@ankur22 ankur22 changed the title Draft: Refactor the BrowserContext class docs for xk6-browser Refactor the BrowserContext class docs for xk6-browser Jul 13, 2022
@ankur22 ankur22 force-pushed the docs/582-xk6-browser branch from b2e2387 to fdaa317 Compare July 14, 2022 15:41
@ankur22 ankur22 force-pushed the docs/441-xk6-browser-browsercontext-class-docs branch from 4ede109 to 5caa62a Compare July 14, 2022 15:41
inancgumus
inancgumus approved these changes Jul 15, 2022
View reviewed changes
Copy link
Member

@inancgumus inancgumus 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 with some suggestions.

And, some questions:

  • What are we going to do about "// FIXME: This is interpreted as 16m40s. Wrong JS->Go conversion." comment in the setDefaultTimeout example (also in some others)? Should we remove it?
  • We might want to kill the link to unroute.
  • Should we remove the missing APIs as we did in your other PRs?

inancgumus
inancgumus reviewed Jul 15, 2022
View reviewed changes
src/data/markdown/docs/30 xk6-browser/01 xk6-browser/02 BrowserContext.md Outdated
| [BrowserContext.newPage()](/javascript-api/xk6-browser/browsercontext/newpage) | Creates a new [page](/javascript-api/xk6-browser/page) inside this `BrowserContext`. |
| <BWIPT id="444"/> [BrowserContext.pages()](/javascript-api/xk6-browser/browsercontext/pages) | Returns a list of [page](/javascript-api/xk6-browser/page)s inside this `BrowserContext`. |
| <BWIPT id="445"/> [BrowserContext.setDefaultNavigationTimeout(timeout)](/javascript-api/xk6-browser/browsercontext/setdefaultnavigationtimeout) | Sets the default navigation timeout in milliseconds. |
| <BWIPT id="456"/> [BrowserContext.setDefaultTimeout(timeout)](/javascript-api/xk6-browser/browsercontext/setdefaulttiontimeout) | Sets the default maximum timeout in milliseconds. |
Copy link
Member

@inancgumus inancgumus Jul 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

By the way, the link should be:

/javascript-api/xk6-browser/browsercontext/setdefaulttimeout/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved by: 5330622

@ankur22
Copy link
Contributor Author

ankur22 commented Jul 19, 2022 •
edited
Loading

  • What are we going to do about "// FIXME: This is interpreted as 16m40s. Wrong JS->Go conversion." comment in the setDefaultTimeout example (also in some others)? Should we remove it?

These have been recorded as issues and the comments removed from the examples:

  • We might want to kill the link to unroute.

The link to this has been removed in the PR -- unless i'm missing something?

  • Should we remove the missing APIs as we did in your other PRs?

Should have been removed already -- did i miss any?

@ankur22 ankur22 force-pushed the docs/441-xk6-browser-browsercontext-class-docs branch 2 times, most recently from 02f0e2f to fcef57f Compare July 19, 2022 15:08
imiric
imiric approved these changes Jul 26, 2022
View reviewed changes
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Nice work!

@ankur22 ankur22 force-pushed the docs/582-xk6-browser branch from 51e5ae0 to 7928525 Compare July 27, 2022 13:14
ankur22 and others added 12 commits July 27, 2022 14:22
@ankur22
Refactor the BrowserContext class for xk6-browser
6fc4336 
- Double checked the workding
- Tested features and linked to issues where necessary.

closes: grafana/xk6-browser#441
@ankur22
Add how BrowserContext is created
086a110
@ankur22
Remove unimplemented features from BrowserContext
2b2ecba
@ankur22
Remove the state column
bf96eb9
@ankur22
Add WIP tooltip and issue numbers
d7720cf
@ankur22
Add the correct links to the types
0049a0a
@ankur22
Add Blockquotes to reference issues
9430565
@ankur22
Remove FIXME and add links to issues
fcf2585
@ankur22 @inancgumus
Apply suggestions from code review
68b1d60 
Co-authored-by: İnanç Gümüş <[email protected]>
@ankur22
Use clear instead of simple tense clears
f2bf49a 
Resolves: https://github.com/grafana/k6-docs/pull/723/files#r922014134
@ankur22
Use belong to clarify possession and link
a1a28b2 
Resolves: https://github.com/grafana/k6-docs/pull/723/files#r922016251
@ankur22
Match option description
68b0fe8 
Resolves: https://github.com/grafana/k6-docs/pull/723/files#r922017118
Resolves: https://github.com/grafana/k6-docs/pull/723/files#r922024138
@ankur22 ankur22 force-pushed the docs/441-xk6-browser-browsercontext-class-docs branch from b9e5ba7 to 0aa0a2a Compare July 27, 2022 13:22
@ankur22 ankur22 merged commit 88389d5 into docs/582-xk6-browser Jul 27, 2022
@ankur22 ankur22 deleted the docs/441-xk6-browser-browsercontext-class-docs branch July 27, 2022 13:23
@inancgumus inancgumus added the Area: browser The browser module label Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@inancgumus inancgumus inancgumus approved these changes

@imiric imiric imiric approved these changes

Assignees
No one assigned
Labels
Area: browser The browser module
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Create docs for BrowserContext class
4 participants
@ankur22 @CLAassistant @inancgumus @imiric