Skip to content

Fix #286: Add Nav::activateParents() that makes parent dropdown active when one of its child items is active#302

Merged
samdark merged 4 commits intomasterfrom
copilot/re-add-activate-parents
Mar 5, 2026
Merged

Fix #286: Add Nav::activateParents() that makes parent dropdown active when one of its child items is active#302
samdark merged 4 commits intomasterfrom
copilot/re-add-activate-parents

Conversation

Copy link
Contributor

Copilot AI commented Mar 5, 2026

Thanks for assigning this issue to me. I'm starting to work on it and will keep this PR's description up to date as I form a plan and make progress.

Original prompt

This section details on the original issue you should resolve

<issue_title>Re-add activateParents</issue_title>
<issue_description>At one point, there was an activateParents function, which seem to have vanished. Is there any plan to bring this back?</issue_description>

Comments on the Issue (you are @copilot in this section)

@terabytesoftw In what case do you need this function and in which widget, of course we can add it if necessary.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: samdark <47294+samdark@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Mar 5, 2026

Codecov Report

❌ Patch coverage is 92.30769% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.96%. Comparing base (1347a46) to head (5c1842b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/Nav.php 92.30% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##              master     #302      +/-   ##
=============================================
- Coverage     100.00%   99.96%   -0.04%     
- Complexity       705      711       +6     
=============================================
  Files             22       22              
  Lines           2922     2934      +12     
=============================================
+ Hits            2922     2933      +11     
- Misses             0        1       +1     

☔ View full report in Codecov by Sentry.
📢 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.

@samdark samdark marked this pull request as ready for review March 5, 2026 20:12
Copilot AI review requested due to automatic review settings March 5, 2026 20:12
@samdark samdark changed the title [WIP] Re-add activateParents function that was removed Fix #286: Add Nav::activateParents() that makes parent dropdown active when one of its child items is active Mar 5, 2026
@samdark samdark merged commit ff9bea0 into master Mar 5, 2026
23 of 25 checks passed
@samdark samdark deleted the copilot/re-add-activate-parents branch March 5, 2026 20:13
Copy link

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

Re-introduces Nav::activateParents() to optionally mark a dropdown toggler as active when one of its child dropdown items is active.

Changes:

  • Add Nav::activateParents() API and internal logic to apply active to dropdown togglers based on active children.
  • Add/extend PHPUnit coverage for the new behavior and immutability.
  • Document the enhancement in the changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/NavTest.php Adds tests validating parent dropdown activation behavior and immutability for activateParents().
src/Nav.php Implements activateParents flag, helper to detect active dropdown children, and applies active class to the dropdown toggler.
CHANGELOG.md Adds an entry documenting the new Nav::activateParents() enhancement.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +108 to +119
/**
* Whether to activate the parent dropdown item when one of its child items is active.
*
* When set to `true`, if a dropdown item's URL matches {@see currentPath}, the dropdown toggle (parent) will also
* receive the `active` CSS class.
*
* @param bool $enabled Whether to activate parent items. Defaults to `false`.
*
* @return self A new instance with the specified activate parents value.
*
* Example usage:
* ```php
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The PHPDoc states “Defaults to false” for $enabled, but the method signature doesn’t provide a default value. To avoid confusing API consumers, either remove the “Defaults…” wording (and rely on the property default) or change the signature to include an explicit default (if that’s intended).

Copilot uses AI. Check for mistakes.
Comment on lines 668 to +673
$dropDownItems = $this->isDropdownActive($items);
$togglerClasses = ['nav-link', 'dropdown-toggle'];

if ($this->activateParents && $this->hasActiveDropdownItem($dropDownItems)) {
$togglerClasses[] = self::NAV_LINK_ACTIVE_CLASS;
}
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

$dropDownItems is a Dropdown instance (returned by isDropdownActive()), but the name reads like an array of items and uses inconsistent casing (“dropDown” vs “dropdown”). Consider renaming to something like $dropdown / $dropdownWithActiveItems for clarity and consistency.

Copilot uses AI. Check for mistakes.
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.

Re-add activateParents

3 participants