Skip to content

feat: add header-keyed table row extraction - #54

Merged
shossain786 merged 2 commits into
seleniumboot:masterfrom
timothytkim:feat/53-locator_rows
Sep 18, 2026
Merged

shossain786 merged 2 commits into
seleniumboot:masterfrom
timothytkim:feat/53-locator_rows

Conversation

@timothytkim

@timothytkim timothytkim commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

What & why

Adds Locator.rows() to extract HTML table rows as header-keyed
List<Map<String, String>> values, including header fallbacks and ragged-row handling.

Closes #53

Type of change

  • Bug fix
  • New feature (opt-in, aligned with the framework philosophy)
  • Documentation
  • Internal refactor / test coverage

Checklist

  • All existing tests pass (mvn clean verify)
  • New behaviour is covered by unit tests
  • No new external dependencies introduced without prior discussion
  • Code follows existing conventions (no new frameworks, minimal abstraction)
  • No breaking changes to @SeleniumBootApi public APIs (or a major version bump is justified below)
  • Docs updated if user-facing behaviour changed (docs-site/)

Notes for reviewers

@shossain786 shossain786 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for taking this on — the header-detection logic (thead → first <tr> with th → first <tr> fallback) and the test coverage for ragged rows, missing thead, and empty tables are solid. Ran the full suite locally, all green.

A couple of things before merge:

Please add Javadoc to rows(). The issue (#53) asked for this explicitly, matching the style of the other terminal actions in the file (e.g. getText(), getAttribute()) — a short description plus a usage example, e.g.:

/**
 * Resolves this locator to a &lt;table&gt; and returns each data row as a
 * header-keyed map. Headers are read from {@code thead th}, falling back to
 * the first {@code <tr>} containing {@code <th>}, then the first {@code <tr>}
 * itself if no {@code <th>} is present anywhere. Cell text is trimmed; short
 * rows are padded with {@code ""}, extra cells are ignored.
 *
 * <pre>
 * List&lt;Map&lt;String, String&gt;&gt; data = $("table#results").rows();
 * assertThat(data.get(0).get("Status")).isEqualTo("Active");
 * </pre>
 */

Move rows() into the "Terminal actions" section (near elements()/getText()), rather than where it currently sits, spliced into the factory-methods block after setTestIdAttribute(). Keeps the file's existing sectioning intact.

Minor: the import java.util.LinkedHashMap; line landed after java.util.stream.Collectors — move it up with the other java.util imports to keep them alphabetical.

Two edge cases worth knowing about, not blockers for this PR, but maybe worth a follow-up issue:

  • Duplicate header text (two columns both named e.g. "Actions") silently collide in the map — last one wins.
  • findElements(By.cssSelector("tr")) matches all descendant <tr>, so a table with a nested <table> inside a cell would pick up the nested rows too.

Also — could you fill in the PR checklist and link Closes #53 in the description? Makes it easier to track against the issue's acceptance criteria.

@timothytkim

timothytkim commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor Author

Updated @shossain786

@shossain786 shossain786 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Javadoc, section placement, and import order all look right now — thanks for the quick turnaround. Re-ran the full suite on the updated branch: LocatorTest 24/24, no regressions elsewhere. Approving.

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.

Locator.rows() — header-keyed table extraction as List<Map<String,String>>

2 participants