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

Fix/dotnet code snippets for text and goto #33382

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Honyii
Copy link
Contributor

@Honyii Honyii commented Oct 31, 2024

This is a dotnet docs update for Page.GetByText and age.GotoAsync

Copy link
Contributor

Test results for "tests 1"

28 failed
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:727:7 › cli codegen › should attribute navigation to click @chromium-ubuntu-22.04-node18
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:796:7 › cli codegen › should middle click @chromium-ubuntu-22.04-node18
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:727:7 › cli codegen › should attribute navigation to click @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:796:7 › cli codegen › should middle click @chromium-ubuntu-22.04-node20
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:727:7 › cli codegen › should attribute navigation to click @chromium-ubuntu-22.04-node22
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:796:7 › cli codegen › should middle click @chromium-ubuntu-22.04-node22
❌ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:727:7 › cli codegen › should attribute navigation to click @firefox-ubuntu-22.04-node18
❌ [firefox-library] › library/inspector/cli-codegen-1.spec.ts:796:7 › cli codegen › should middle click @firefox-ubuntu-22.04-node18
❌ [installation tests] › playwright-electron-should-work.spec.ts:21:5 › electron should work @package-installations-macos-latest
❌ [installation tests] › playwright-test-plugin.spec.ts:35:5 › npm: @playwright/test plugin should work @package-installations-macos-latest @playwright/test
❌ [installation tests] › playwright-test-plugin.spec.ts:49:5 › pnpm: @playwright/test plugin should work @package-installations-macos-latest @playwright/test
❌ [installation tests] › playwright-test-plugin.spec.ts:63:5 › yarn: @playwright/test plugin should work @package-installations-macos-latest @playwright/test
❌ [installation tests] › typescript-types.spec.ts:18:5 › typescript types should work @package-installations-macos-latest
❌ [installation tests] › playwright-electron-should-work.spec.ts:21:5 › electron should work @package-installations-ubuntu-latest
❌ [installation tests] › playwright-test-plugin.spec.ts:35:5 › npm: @playwright/test plugin should work @package-installations-ubuntu-latest @playwright/test
❌ [installation tests] › playwright-test-plugin.spec.ts:49:5 › pnpm: @playwright/test plugin should work @package-installations-ubuntu-latest @playwright/test
❌ [installation tests] › playwright-test-plugin.spec.ts:63:5 › yarn: @playwright/test plugin should work @package-installations-ubuntu-latest @playwright/test
❌ [installation tests] › typescript-types.spec.ts:18:5 › typescript types should work @package-installations-ubuntu-latest
❌ [installation tests] › playwright-electron-should-work.spec.ts:21:5 › electron should work @package-installations-windows-latest
❌ [installation tests] › playwright-test-plugin.spec.ts:35:5 › npm: @playwright/test plugin should work @package-installations-windows-latest @playwright/test
❌ [installation tests] › playwright-test-plugin.spec.ts:49:5 › pnpm: @playwright/test plugin should work @package-installations-windows-latest @playwright/test
❌ [installation tests] › playwright-test-plugin.spec.ts:63:5 › yarn: @playwright/test plugin should work @package-installations-windows-latest @playwright/test
❌ [installation tests] › typescript-types.spec.ts:18:5 › typescript types should work @package-installations-windows-latest
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:727:7 › cli codegen › should attribute navigation to click @ubuntu-20.04-chromium-tip-of-tree
❌ [chromium-library] › library/inspector/cli-codegen-1.spec.ts:796:7 › cli codegen › should middle click @ubuntu-20.04-chromium-tip-of-tree
❌ [webkit-library] › library/inspector/cli-codegen-1.spec.ts:727:7 › cli codegen › should attribute navigation to click @webkit-ubuntu-22.04-node18
❌ [webkit-library] › library/inspector/cli-codegen-1.spec.ts:796:7 › cli codegen › should middle click @webkit-ubuntu-22.04-node18
❌ [playwright-test] › update-aria-snapshot.spec.ts:167:5 › should update missing snapshots in tsx @windows-latest-node18-1

35361 passed, 625 skipped
✔️✔️✔️

Merge workflow run.

@Skn0tt
Copy link
Member

Skn0tt commented Oct 31, 2024

I think the snippets are correct as they are. Could you outline what's the problem in your opinion?

@Honyii
Copy link
Contributor Author

Honyii commented Oct 31, 2024 via email

@Honyii
Copy link
Contributor Author

Honyii commented Oct 31, 2024 via email

@Skn0tt
Copy link
Member

Skn0tt commented Oct 31, 2024

https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/coding-style/identifier-names#naming-conventions says that local variables should be in camelCase, and the snippets this PR updates are all assuming page to be a local variable. Am I missing something?

@Honyii
Copy link
Contributor Author

Honyii commented Oct 31, 2024 via email

@Honyii
Copy link
Contributor Author

Honyii commented Oct 31, 2024 via email

Copy link
Member

@mxschmitt mxschmitt left a comment

Choose a reason for hiding this comment

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

Hello! Please revert the changes inside docs/src/api folder since they target the library where page is usually lowercase and the rest are guides which target Microsoft.Playwright.NUnit etc.

Also now a lot of methods use now mixed casing, some a bit of Page and some a bit of page. e.g. looking at input.md.

@Honyii
Copy link
Contributor Author

Honyii commented Nov 1, 2024 via email

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.

3 participants