[javoconsultant]

Description

Follow-up to #3260, which fixed points 1-4 of openedx/openedx-core#821. QA then reported that the hover effect added there "appears only when the pointer at the top part of the row. When the pointer is at the left, right, or bottom part of the row, hover doesn't appear. And the row is clickable only at the top part of the row."

A group node in the tree paints its hover and selection tints across its whole grey envelope - header strip, the padding around it, and the nested children it wraps. But role="treeitem", the click/keyboard handlers and the --selectable/--selected styling all lived on the .competency-row header <div> inside it, which is only as tall as its own text. The tinted box and the hit area were two different boxes.

Hit-testing every pixel of each node in a browser quantified it:

node	dead on the pointer, before	after
QA706 Framework Root (outer group)	33.3%	0%
QA706 Group Analysis (group)	21.6%	0%
QA706 Leaf Data Interpretation (leaf)	0.2%	0%

Leaf nodes, whose <li> and row are the same box, were already fine.

Changes

• CompetencyTreeItem.tsx - the <li> itself is now the treeitem and the click target, so the hit area is the tinted envelope. handleSelect stops propagation, since a child's <li> sits inside its ancestors' and would otherwise select every group above it on the way up.
• CompetencyTree.scss - fill, border, hover tint and the teal selected accent move onto the <li>. Hover uses :not(:has(li:hover)) so only the innermost hovered node lights up.

This also drops the old role="none" workaround: a treeitem as a direct child of its tree/group <ul> is just the plain ARIA tree structure. And it lets the separate group-envelope tint, the group-level ::before and the row-level ::before's display: none override collapse into a single .competency-node--selected rule - the stylesheet is a net 36 lines shorter.

Testing instructions

1. Go to Taxonomy → a taxonomy with the Competency Management page enabled, with nested competencies, and click Expand All.
2. Move the pointer around a group's grey envelope - its left and right padding strips beside a child, the area below the last child, and the gaps between children. The group should highlight from all of them, not just from its header text line, and the cursor should be a pointer throughout.
3. Move onto a child row inside that group. Only the child should highlight; its parent's tint should drop away.
4. Click any of those previously-dead areas and confirm the group gets selected (teal bar, right-hand course panel opens for it).
5. Click a nested child and confirm only that child becomes selected, never one of its ancestor groups.
6. Tab through the tree and confirm each node is still focusable and that Enter/Space select it, while the chevron still only expands/collapses.

Other information

Verified in headless Chrome against the pre-change build:

• Real mouse events driven over CDP now tint the group from its left, right and bottom padding (previously nothing at all), while a child still tints only itself.
• The resting, selected-leaf and selected-group renders are byte-identical to the previous build - this is a hit-area fix, not a visual change.
• axe-core reports 0 violations on the new markup, same as before.

314 taxonomy tests pass, including two new regression tests: one asserting the group's treeitem wraps its nested children, one asserting a click or Enter on a nested leaf selects only that leaf and never its ancestor groups. dprint, oxlint --type-aware --deny-warnings, stylelint and tsc --noEmit are all clean.

No new dependencies, migrations, or i18n changes.

Supporting information

openedx/openedx-core#821
#3260

Best Practices Checklist

We're trying to move away from some deprecated patterns in this codebase. Please
check if your PR meets these recommendations before asking for a review:

• Any new files are using TypeScript (.ts, .tsx).
• Avoid propTypes and defaultProps in any new or modified code.
• Tests should use the helpers in src/testUtils.tsx (specifically initializeMocks)
• Do not add new fields to the Redux state/store. Use React Context to share state among multiple components.
• Use React Query to load data from REST APIs. See any apiHooks.ts in this repo for examples.
• All new i18n messages in messages.ts files have a description for translators to use.
• Avoid using ../ in import paths. To import from parent folders, use @src, e.g. import { initializeMocks } from '@src/testUtils'; instead of from '../../../../testUtils'

🤖 Generated with Claude Code

[openedx-webhooks]

Thanks for the pull request, @javoconsultant!

This repository is currently maintained by @bradenmacdonald.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.12%. Comparing base (40ebb41) to head (fc7e539).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff            @@
##           master    #3291    +/-   ##
========================================
  Coverage   96.12%   96.12%            
========================================
  Files        1428     1428            
  Lines       34717    34724     +7     
  Branches     8295     8022   -273     
========================================
+ Hits        33372    33379     +7     
  Misses       1304     1304            
  Partials       41       41            

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mgwozdz-unicon]

Claude and I reviewed this together, and we have a few requested changes:

1. Group treeitems now take their accessible name from their whole subtree (CompetencyTreeItem.tsx, the <li role="treeitem">)

A treeitem with no aria-label or aria-labelledby computes its accessible name from all of its text content. Now that a group's nested <ul role="group"> lives inside the group's own <li>, every descendant's text becomes part of the group's name. Running computeAccessibleName from dom-accessibility-api (the library Testing Library uses for getByRole(..., { name })) against the nestedTagsResponse fixture after Expand All gives:

• Root A: "Root A Competency ID: EXT-001 Group A1 Leaf A1a Competency ID: EXT-003"
• Group A1: "Group A1 Leaf A1a Competency ID: EXT-003"

Before this PR the treeitem was the row <div>, so its name was only that row's own text. On a real taxonomy, a screen reader focusing a top-level group could read out the entire subtree. I have only checked this in jsdom, not in a real browser and screen reader, and axe does not flag it because a long name is not a rule violation. Could you point the treeitem's name at its own row instead, e.g. aria-labelledby referencing ids on the .competency-row__label span and the sr-only Competency ID span (via useId), so the ID is still announced?

2. The new "selects only the innermost node clicked" test is redundant (CompetencyTree.test.tsx, last test)

The test does catch ancestor selection, through toHaveBeenCalledTimes(1): without stopPropagation the click bubbles to Group A1's and Root A's <li> handlers and the mock is called three times. But three existing tests already fail the same way. I removed the event.stopPropagation() line in handleSelect and re-ran the file: the leaf-click test, the group-click test, and the leaf Enter/Space test all fail alongside the new one, because each already asserts a call count of 1. Could you drop the new test? If you'd like the intent stated explicitly, adding expect(onSelectCompetency).not.toHaveBeenCalledWith(expect.objectContaining({ id: 2 })) to the existing leaf-click test would cover it in one line.

3. Truncated comment (CompetencyTree.scss, above .competency-node--selected::before)

The comment now ends mid-sentence at "where it meets the shared". The edit removed the line that finished it ("border-radius arc").

[javoconsultant]

Claude and I reviewed this together, and we have a few requested changes:

1. Group treeitems now take their accessible name from their whole subtree (CompetencyTreeItem.tsx, the <li role="treeitem">)

@mgwozdz-unicon i've addressed your comments

[mgwozdz-unicon]

Thank you, approving. Just one nit. Also, Claude says: The PR description still says "two new regression tests" and describes the "click or Enter on a nested leaf selects only that leaf" test, which has since been removed. It would be worth updating it to mention the accessible-name test instead.

[arbrandes]

👍🏼