Skip to content

test: update ViewIT selectors for fc-applayout shadow DOM - #151

Merged
paodb merged 1 commit into
masterfrom
test/fix-viewit
Oct 26, 2026
Merged

paodb merged 1 commit into
masterfrom
test/fix-viewit

Conversation

@javier-godoy

@javier-godoy javier-godoy commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

ViewIT was written for the 2.x/3.x DOM and has failed since the component moved to fc-applayout (see #149).

  • Open applayout-full directly. The root route now forwards to the tabbed demo, which shows the layout in an iframe.
  • Look up app-header and app-drawer inside fc-applayout, which renders them in its shadow root.
  • Find the logo, title and settings button as slotted children of fc-applayout ([slot='title'], [slot='toolbar']), not inside app-toolbar.
  • Find menu items as [slot='menu'] children of fc-applayout, so the non-empty check counts real items again, not just the <slot> element.

mvn verify -Pintegration-tests passes locally: ViewIT and SerializationTest each ran 1 test.

Close #149

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Updated integration checks to verify the full app layout, including its header, drawer, title and logo, settings button, and menu items.

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 25924fb9-e852-45bf-a189-447e8e4cfd6c
Commits

Reviewing files that changed from the base of the PR and between 6dac9a5 and 2e35a7c.

Files selected for processing: 1
  • src/test/java/com/flowingcode/addons/applayout/integration/ViewIT.java

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

ViewIT now opens the applayout-full view. The test locates fc-applayout, checks its custom-element state, and queries its header, drawer, title, toolbar, and menu elements using the current DOM structure.

Changes

ViewIT integration test

Layer / File(s) Summary
Update integration test selectors
src/test/java/com/flowingcode/addons/applayout/integration/ViewIT.java
The test opens applayout-full, scopes header and drawer queries to fc-applayout, and locates title, toolbar, and menu elements through their slots.

Priority: Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: Minimal · up to 2e35a

The change is limited to updating integration-test selectors to the current demo DOM, with no identified merge-blocking risk.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: updating ViewIT selectors for the current fc-applayout shadow DOM.
Linked Issues check Passed Linked issue #149 has one active coding requirement: update ViewIT to the current fc-applayout and applayout-full DOM. The diff changes only src/test/java/com/flowingcode/addons/applayout/integration/…
Out of Scope Changes check Passed The whole-PR diff against the merge base touches one file: ViewIT.java. Every hunk changes a selector, a slot query, the constructor view path, or a related assertion in componentWorks. Each change se…
Full details: Linked Issues check

Explanation

Linked issue #149 has one active coding requirement: update ViewIT to the current fc-applayout and applayout-full DOM. The diff changes only src/test/java/com/flowingcode/addons/applayout/integration/ViewIT.java, and it covers each point in #149:
(1) Open the page directly: the new constructor calls super("applayout-full"), so the test no longer starts at the root route that forwards to the iframe-based tabbed demo.
(2) Shadow DOM: $("fc-applayout") is located and checked for upgrade. app-header and app-drawer are then looked up inside that layout (layout.$(...)).
(3) Slotted children: the logo is checked as img.applogo[slot='title'], the title as div[slot='title'], and the settings button as paper-icon-button[slot='toolbar'][role='button'][icon='settings'], all searched on the layout. This matches the slot names used in fc-fusion-layout.ts and the #149 description.
(4) Menu items: the non-empty check and the upgrade loop now use layout.findElements(By.cssSelector("[slot='menu']")) instead of all children of paper-listbox. The menu-icon button check is kept, as #149 states.
Limits of this review: the run result (mvn verify -Pintegration-tests passing) is the author's reported local result. This review did not execute the test. The [slot='menu'] check counts the elements assigned to the menu slot. If an item is wrapped in a slot='menu' container, the check counts the wrapper. The change matches the selector that #149 requests, so no concrete gap was found.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base touches one file: ViewIT.java. Every hunk changes a selector, a slot query, the constructor view path, or a related assertion in componentWorks. Each change serves the #149 requirement to update ViewIT to the current DOM. No unrelated production code, build configuration, or other test was changed. No out-of-scope change was found.

Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@javier-godoy
javier-godoy marked this pull request as ready for review October 26, 2026 17:31
@paodb
paodb merged commit fcc3ef5 into master Oct 26, 2026
7 checks passed
@paodb
paodb deleted the test/fix-viewit branch October 26, 2026 18:07
@github-project-automation github-project-automation Bot moved this from To Do to Pending release in Flowing Code Addons Oct 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Pending release

Development

Successfully merging this pull request may close these issues.

ViewIT is out of date and fails against the current demo

2 participants