Skip to content

(#513) Add function to find row in table by text #514

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

Open
alexaveldanez wants to merge 1 commit into
base: main
Choose a base branch
from
Open

(#513) Add function to find row in table by text #514

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions packages/playwright/global/functions/find-row-in-table.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { Locator, Page } from '@playwright/test';

// functions
import { waitForTables } from '@choco-playwright/functions/wait-for-tables';

/**
* Finds the first row with specific text in a table and returns the locator for that row.
* @function findRowInTable
* @param {Locator} table - The locator for the table.
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The JSDoc documents a table parameter that is never used in the function implementation. This documentation is misleading and should either be removed (if the parameter is removed) or updated to reflect the actual usage. The current implementation searches for rows in the entire page, not scoped to a specific table.

Copilot uses AI. Check for mistakes.
* @param {Page} page - The Playwright page instance.
* @param {string} text - The text you are searching for in the table.
* @returns {Locator} - Locator for the row.
*/

export const findRowInTable = async(table: Locator, page: Page, text: string) => {
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The table parameter is declared but never used in the function body. All locator operations use page.locator() directly instead of scoping to the provided table. Either remove this parameter if it's not needed, or use it to scope the row search to a specific table (e.g., table.locator('tr', ...)). Based on the function's purpose, it appears the parameter should be removed since the function searches the entire page.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

Missing space between async and the opening parenthesis. This should be async (table: Locator, page: Page, text: string) to match the formatting convention used in other functions in this codebase (see convert-to-file-path.ts:15, expand-section.ts:17, wait-for-tables.ts:11).

Suggested change
export const findRowInTable = async(table: Locator, page: Page, text: string) => {
export const findRowInTable = async (table: Locator, page: Page, text: string) => {

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The function signature is missing an explicit return type. According to the codebase convention, all async functions should specify their return type (see examples in convert-to-file-path.ts:15, expand-section.ts:20, wait-for-tables.ts:11). Add : Promise<Locator> after the parameter list to match this pattern.

Copilot uses AI. Check for mistakes.
const btnNext = page.getByRole('link', { name: 'Next' });

while(true) {
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

Missing space after while. The code has while(true) but should be while (true) to match standard formatting conventions.

Suggested change
while(true) {
while (true) {

Copilot uses AI. Check for mistakes.
const row = page.locator('tr', { has: page.locator('td', { hasText: text })});

if (await row.count() > 0) {
return row.first();
}

const ariaDisabled = await btnNext.getAttribute('aria-disabled');

if (ariaDisabled) {
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The condition if (ariaDisabled) checks for any truthy value, but getAttribute() returns string | null. This means the check will treat the string "false" as truthy and incorrectly break the loop. The condition should explicitly check for the value "true": if (ariaDisabled === 'true') to properly determine when the Next button is disabled.

Suggested change
if (ariaDisabled) {
if (ariaDisabled === 'true') {

Copilot uses AI. Check for mistakes.
break;
}

await btnNext.click();
await waitForTables(page);
Comment on lines +18 to +32
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The function could enter an infinite loop if the "Next" button doesn't have an aria-disabled attribute when it reaches the last page. Consider adding a timeout or maximum page count as a safety mechanism. Additionally, if the Next button doesn't exist on the page at all, the getAttribute() call on line 25 could throw an error. Add existence checks or error handling to make the function more robust.

Copilot uses AI. Check for mistakes.
}

throw new Error(`${text} not found in the table after checking all pages.`);
Copy link

Copilot AI Jan 30, 2026

Choose a reason for hiding this comment

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

The error message uses template literal syntax but doesn't escape the text parameter, which could lead to confusing error messages if the search text contains special characters or is very long. Consider wrapping the text in quotes for clarity: throw new Error(\Row with text "${text}" not found in the table after checking all pages.`);`

Suggested change
throw new Error(`${text} not found in the table after checking all pages.`);
throw new Error(`Row with text "${text}" not found in the table after checking all pages.`);

Copilot uses AI. Check for mistakes.
};
Loading