Skip to content

Show only selectable sorting options in filters - #256

Merged
farmio merged 2 commits into
XKNX:mainfrom
philippwaller:feat/disable-sort-option
Aug 7, 2025
Merged

Show only selectable sorting options in filters#256
farmio merged 2 commits into
XKNX:mainfrom
philippwaller:feat/disable-sort-option

Conversation

@philippwaller

Copy link
Copy Markdown
Contributor

This PR ensures that only sorting options relevant to the current context are displayed in the filters. The Sort by Filter Count option is now only shown when filter options are currently selected or when this sorting option is already active.

Copilot AI review requested due to automatic review settings August 6, 2025 16:41

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the user experience by dynamically controlling the visibility of sorting options in group monitor filters. Specifically, the "Sort by Filter Count" option is now only shown when filters are active or when this sorting option is currently selected, preventing confusion when the sorting option would be meaningless.

Key Changes

  • Added conditional logic to enable/disable the "Sort by Filter Count" option based on filter state
  • Enhanced the sort menu system to support disabled states for sort options
  • Refactored sort menu item rendering to use a unified approach for both standard and custom fields

Reviewed Changes

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

File Description
group-monitor-view.ts Added filter component queries and conditional logic to control "Sort by Filter Count" availability
knx-sort-menu-item.ts Added disabled state support with proper styling and interaction prevention
knx-list-filter.ts Added sortDisabled configuration option and unified sort menu item rendering

Comment thread src/features/group-monitor/views/group-monitor-view.ts
Comment thread src/features/group-monitor/views/group-monitor-view.ts
Comment thread src/components/knx-sort-menu-item.ts Outdated
@codecov

codecov Bot commented Aug 6, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 91 lines in your changes missing coverage. Please review.
✅ Project coverage is 3.32%. Comparing base (271075f) to head (fb76b1a).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/components/knx-sort-menu-item.ts 0.00% 47 Missing ⚠️
...rc/components/data-table/filter/knx-list-filter.ts 0.00% 31 Missing ⚠️
...features/group-monitor/views/group-monitor-view.ts 0.00% 13 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##            main    #256      +/-   ##
========================================
+ Coverage   3.16%   3.32%   +0.15%     
========================================
  Files         55      55              
  Lines      10284   10328      +44     
  Branches     148     156       +8     
========================================
+ Hits         326     343      +17     
- Misses      9958    9985      +27     

☔ 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…s: none

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@philippwaller

Copy link
Copy Markdown
Contributor Author

With more refactoring ahead, I’d prefer to skip writing tests for now, as they could quickly become outdated. I’ll include them with the upcoming changes once things have settled.

@farmio

farmio commented Aug 7, 2025

Copy link
Copy Markdown
Member

It was you who introduced tests and coverage to this repo... so fine for me 😉

@farmio
farmio merged commit 5c6b2b2 into XKNX:main Aug 7, 2025
5 of 6 checks passed
@philippwaller
philippwaller deleted the feat/disable-sort-option branch August 12, 2025 18:12
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