Skip to content

Bug rux tree select multiple#1410

Merged
brothhammer merged 4 commits intomainfrom
bug-rux-tree-select-multiple
Jul 15, 2025
Merged

Bug rux tree select multiple#1410
brothhammer merged 4 commits intomainfrom
bug-rux-tree-select-multiple

Conversation

@brothhammer
Copy link
Copy Markdown
Contributor

@brothhammer brothhammer commented Jul 15, 2025

Brief Description

We want to allow selections to stay active when there are multiple trees on a page.

JIRA Link

https://rocketcom.atlassian.net/jira/software/projects/AP/boards/119?assignee=712020%3A6a79a1e3-7381-4656-9516-c09792ad18be%2C60e2c268bd7f5d006886d1bf%2C712020%3A58e256f4-325a-4523-ae75-89831b3bbeab&selectedIssue=AP-561

Related Issue

General Notes

Run the below command from the web-components directory to run just the tree tests
npx playwright test src/components/rux-tree/test/tree.spec.ts

There are two trees in the index.html playground you can test locally with
npm run start from the web-components directory

Motivation and Context

This came from a git hub issue report.
#1407

Issues and Limitations

Types of changes

  • Bug fix
  • New feature
  • Breaking change

Checklist

  • This PR adds or removes a Storybook story.
  • I have added tests to cover my changes.
  • Regressions are passing and/or failures are documented
  • Changes have been checked in evergreen browsers

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jul 15, 2025

⚠️ No Changeset found

Latest commit: ed48aa8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link
Copy Markdown

netlify bot commented Jul 15, 2025

Deploy Preview for astro-preview ready!

Name Link
🔨 Latest commit ed48aa8
🔍 Latest deploy log https://app.netlify.com/projects/astro-preview/deploys/6876b726a0cb440007a0542f
😎 Deploy Preview https://deploy-preview-1410--astro-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@brothhammer brothhammer requested a review from Copilot July 15, 2025 20:16
@netlify
Copy link
Copy Markdown

netlify bot commented Jul 15, 2025

Deploy Preview for astro-stencil ready!

Name Link
🔨 Latest commit ed48aa8
🔍 Latest deploy log https://app.netlify.com/projects/astro-stencil/deploys/6876b726d393f30007e4c7e7
😎 Deploy Preview https://deploy-preview-1410--astro-stencil.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Demonstrates independent selection behavior for multiple tree instances by scoping selection logic to each rux-tree, adds a sample in index.html, and covers the change with a new Playwright test.

  • Scope selection handling to the current tree element rather than the entire document
  • Add a side-by-side example of two trees in the documentation
  • Introduce a Playwright test to verify independent selections across multiple trees

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/web-components/src/index.html Added a section showing two rux-tree instances side-by-side to illustrate independent selections
packages/web-components/src/components/rux-tree/test/tree.spec.ts Added a new test (allows independent selections across multiple trees) to validate selection isolation
packages/web-components/src/components/rux-tree/rux-tree.tsx Updated _handleNodeSelected to query only nodes within the current tree (this.el) instead of document
Comments suppressed due to low confidence (2)

packages/web-components/src/components/rux-tree/rux-tree.tsx:54

  • [nitpick] The variable name allNodes is ambiguous; consider renaming it to treeNodes or localNodes to clarify that it only refers to nodes within this tree instance.
        const allNodes = this.el.querySelectorAll('rux-tree-node')

packages/web-components/src/index.html:111

  • Add an aria-label (or similar accessible name) to each rux-tree to distinguish them for assistive technologies, e.g., aria-label="Navigation Tree" and aria-label="File System Tree".
                        <rux-tree>

Copy link
Copy Markdown
Contributor

@FMorrison87 FMorrison87 left a comment

Choose a reason for hiding this comment

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

lgtm!

@brothhammer brothhammer merged commit bb11a73 into main Jul 15, 2025
7 checks passed
@brothhammer brothhammer deleted the bug-rux-tree-select-multiple branch July 15, 2025 21:54
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