feat(plugins): Implement auto-hide functionality for empty plugin tabs#1578
feat(plugins): Implement auto-hide functionality for empty plugin tabs#1578
Conversation
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant Script as Badge Fetch Script
participant AutoHide as autoHideEmptyTabs()
participant DOM as DOM (nav, panes)
participant Bootstrap as Bootstrap Tabs
participant DataTable as Deferred DataTable Init
Script->>AutoHide: call autoHideEmptyTabs(counts, prefixes)
AutoHide->>DOM: read badge counts for plugins & sub-tabs
AutoHide->>DOM: hide plugin nav items/panes with total==0
AutoHide->>DOM: hide sub-tabs with count==0, remove .active if needed
AutoHide->>Bootstrap: if hidden active sub-tab -> Bootstrap.tab('show') first visible
Bootstrap->>DataTable: shown.bs.tab event triggers deferred DataTable init
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@front/pluginsCore.php`:
- Around line 446-447: The code is forcibly calling $li.show() and $pane.show(),
which breaks tab visibility by overriding CSS-based show/hide controlled via the
'active' class; remove these unconditional .show() calls (references:
$li.show(), $pane.show() in the pluginsCore tab rendering logic) and instead
rely on toggling the 'active' class on the tab element and its corresponding
.tab-pane (or only call .show() conditionally when the element has the 'active'
class). Also remove or change the duplicate .show() calls around the sub-pane
rendering (the same calls at lines noted in the review) so only the active
tab/pane is displayed.
- Around line 442-443: When hiding a tab ensure you remove the active class from
both the pane and its corresponding list item: remove 'active' from $li as well
as calling $pane.removeClass('active').hide(). For fallback activation of a
sub-tab, do not call addClass('active') directly; instead activate the tab via
Bootstrap's tab lifecycle (i.e. call .tab('show') on the tab link for the
fallback sub-tab) so the shown.bs.tab handlers that perform deferred DataTable
initialization (the handlers referenced around shown.bs.tab) run. Update the
code paths that hide sub-tabs ($li, $pane) and the fallback activation logic
that currently uses addClass('active') to use .tab('show') on the tab anchor
element.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d37d6f98-8581-4f78-b0f6-67cdb725f8e7
📒 Files selected for processing (1)
front/pluginsCore.php
…by ensuring proper visibility handling and Bootstrap integration
There was a problem hiding this comment.
♻️ Duplicate comments (2)
front/pluginsCore.php (2)
461-464:⚠️ Potential issue | 🟡 MinorRemove
activeclass from hidden sub-tab<li>element.When hiding a sub-tab with zero count,
$subLiretains theactiveclass even though it's hidden. This is inconsistent with line 442 where bothremoveClass('active')andhide()are called on the main tab. Hidden elements retainingactivecan cause multiple tabs to appear active after a filter change.🐛 Proposed fix
if (st.count === 0) { if ($subLi.hasClass('active')) activeSubHidden = true; - $subLi.hide(); + $subLi.removeClass('active').hide(); $subPane.removeClass('active').css('display', ''); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/pluginsCore.php` around lines 461 - 464, When st.count === 0, the hidden sub-tab <li> ($subLi) still keeps the 'active' class; update the branch handling zero count so that you call $subLi.removeClass('active') before or after $subLi.hide(), and keep the existing $subPane.removeClass('active'). This change should be applied in the block that checks if (st.count === 0) and references $subLi and $subPane to ensure no hidden <li> retains the 'active' class.
487-493:⚠️ Potential issue | 🟠 MajorRemove manual
addClass('active')calls—they prevent.tab('show')from firing theshown.bs.tabevent.Bootstrap's
.tab('show')method returns early without action when the parent<li>already has theactiveclass. By calling$firstVisibleLi.addClass('active')at line 487 before.tab('show')at line 492, the tab lifecycle is bypassed andshown.bs.tabnever fires. This breaks deferred DataTable initialization that depends on that event (lines 529-531).Let
.tab('show')handle all class manipulations—it will addactiveto both the<li>and pane automatically.🐛 Proposed fix
// If the active left-nav tab was hidden, activate the first visible one const $activeLi = $(`#tabs-location li.active:visible`); if ($activeLi.length === 0) { const $firstVisibleLi = $(`#tabs-location li:visible`).first(); if ($firstVisibleLi.length) { - $firstVisibleLi.addClass('active'); - const targetPrefix = $firstVisibleLi.find('a').attr('href')?.replace('#', ''); - if (targetPrefix) { - $(`#tabs-content-location > #${targetPrefix}`).addClass('active'); - // Trigger shown.bs.tab so deferred DataTables initialize - $firstVisibleLi.find('a').tab('show'); - } + // Let Bootstrap's tab lifecycle handle active class and fire shown.bs.tab + $firstVisibleLi.find('a[data-toggle="tab"]').tab('show'); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@front/pluginsCore.php` around lines 487 - 493, The code manually sets active classes which prevents Bootstrap's .tab('show') from running its lifecycle and firing shown.bs.tab; remove the manual activations by deleting the $firstVisibleLi.addClass('active') and $(`#tabs-content-location > #${targetPrefix}`).addClass('active') lines, keep the targetPrefix resolution and call $firstVisibleLi.find('a').tab('show') so Bootstrap handles adding/removing active classes and triggers shown.bs.tab (references: $firstVisibleLi, targetPrefix, .tab('show'), shown.bs.tab).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@front/pluginsCore.php`:
- Around line 461-464: When st.count === 0, the hidden sub-tab <li> ($subLi)
still keeps the 'active' class; update the branch handling zero count so that
you call $subLi.removeClass('active') before or after $subLi.hide(), and keep
the existing $subPane.removeClass('active'). This change should be applied in
the block that checks if (st.count === 0) and references $subLi and $subPane to
ensure no hidden <li> retains the 'active' class.
- Around line 487-493: The code manually sets active classes which prevents
Bootstrap's .tab('show') from running its lifecycle and firing shown.bs.tab;
remove the manual activations by deleting the $firstVisibleLi.addClass('active')
and $(`#tabs-content-location > #${targetPrefix}`).addClass('active') lines,
keep the targetPrefix resolution and call $firstVisibleLi.find('a').tab('show')
so Bootstrap handles adding/removing active classes and triggers shown.bs.tab
(references: $firstVisibleLi, targetPrefix, .tab('show'), shown.bs.tab).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e116d2d9-bf09-4d68-9fbc-4b8d793b0101
📒 Files selected for processing (1)
front/pluginsCore.php
1 participant
Summary by CodeRabbit