Filter control for what models on main tab#344
Filter control for what models on main tab#344mitchcapper wants to merge 1 commit intorobinebers:mainfrom
Conversation
mitchcapper
requested review from
davidarny,
robinebers and
validatedev
as code owners
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Adds per-provider controls to hide specific “overview” model lines on the main Overview tab, while keeping provider-specific tabs unchanged. This integrates into the existing plugin settings persistence and app view wiring.
Changes:
- Persist per-provider
disabledOverviewLabelsin plugin settings and plumb it through app state/hooks. - Add Settings UI (for enabled providers) to toggle visibility of each
scope: "overview"line. - Filter out disabled overview labels when rendering
ProviderCardon the Overview page.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pages/settings.tsx | Adds UI to toggle which overview-scoped lines are shown per provider. |
| src/pages/overview.tsx | Passes disabledOverviewLabels down to ProviderCard for filtering. |
| src/lib/settings.ts | Extends persisted plugin settings with disabledOverviewLabels, including normalization + equality. |
| src/lib/plugin-types.ts | Extends PluginDisplayState with optional disabledOverviewLabels. |
| src/hooks/app/use-settings-plugin-list.ts | Includes manifest lines + disabled overview labels in settings plugin list state. |
| src/hooks/app/use-settings-plugin-actions.ts | Adds action to toggle a disabled overview label and persist it. |
| src/hooks/app/use-app-plugin-views.ts | Threads disabledOverviewLabels into display plugin view state. |
| src/components/provider-card.tsx | Filters overview lines by disabledOverviewLabels when in overview scope. |
| src/components/app/app-content.tsx | Wires onToggleOverviewLabel through to Settings page. |
| src/App.tsx | Connects new settings action handler into AppContent props. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -91,6 +93,7 @@ export async function loadPluginSettings(): Promise<PluginSettings> { | |||
| return { | |||
| order: Array.isArray(stored.order) ? stored.order : [], | |||
| disabled: Array.isArray(stored.disabled) ? stored.disabled : [], | |||
| disabledOverviewLabels: stored.disabledOverviewLabels || {}, | |||
| }; | |||
There was a problem hiding this comment.
loadPluginSettings() now always returns disabledOverviewLabels (defaulting to {}), which changes the shape of the returned settings object compared to previous versions. This will break any strict equality expectations (e.g. tests or code paths expecting { order, disabled } only) and also means savePluginSettings({order,disabled}) followed by loadPluginSettings() will not round-trip to the same object. Consider either (a) making disabledOverviewLabels non-optional everywhere and updating expectations accordingly, or (b) only including disabledOverviewLabels in the return value when it exists in storage (and letting callers apply defaults).
| const disabledOverviewLabels: Record<string, string[]> = {}; | ||
| if (settings.disabledOverviewLabels) { | ||
| for (const [id, labels] of Object.entries(settings.disabledOverviewLabels)) { | ||
| if (knownSet.has(id) && Array.isArray(labels)) { | ||
| disabledOverviewLabels[id] = labels; | ||
| } | ||
| } |
There was a problem hiding this comment.
normalizePluginSettings() copies disabledOverviewLabels values as-is when they are arrays, but doesn’t sanitize the array contents (e.g. non-string entries) or normalize ordering / deduplicate. Since arePluginSettingsEqual() performs an order-sensitive comparison, semantically identical label sets with different ordering will be treated as unequal. Consider filtering to strings and sorting (and optionally removing duplicates) during normalization to keep settings deterministic and avoid unnecessary re-saves.
| const nextSettings: PluginSettings = { | ||
| ...pluginSettings, | ||
| disabledOverviewLabels: { | ||
| ...pluginSettings.disabledOverviewLabels, | ||
| [pluginId]: nextDisabled, | ||
| }, |
There was a problem hiding this comment.
When the last disabled label is re-enabled, handleToggleOverviewLabel will persist an empty array for that plugin id (e.g. { disabledOverviewLabels: { [pluginId]: [] } }). This is semantically equivalent to omitting the key, but arePluginSettingsEqual() will treat {} vs {id: []} as different and it also bloats the stored settings. Consider deleting the key when nextDisabled.length === 0 instead of storing an empty list.
| const nextSettings: PluginSettings = { | |
| ...pluginSettings, | |
| disabledOverviewLabels: { | |
| ...pluginSettings.disabledOverviewLabels, | |
| [pluginId]: nextDisabled, | |
| }, | |
| const nextDisabledOverviewLabels = { | |
| ...(pluginSettings.disabledOverviewLabels ?? {}), | |
| } | |
| if (nextDisabled.length === 0) { | |
| delete nextDisabledOverviewLabels[pluginId] | |
| } else { | |
| nextDisabledOverviewLabels[pluginId] = nextDisabled | |
| } | |
| const nextSettings: PluginSettings = { | |
| ...pluginSettings, | |
| disabledOverviewLabels: nextDisabledOverviewLabels, |
| export type SettingsPluginState = { | ||
| id: string | ||
| name: string | ||
| enabled: boolean | ||
| lines: ManifestLine[] | ||
| disabledOverviewLabels: string[] | ||
| } |
There was a problem hiding this comment.
useSettingsPluginList now returns additional required fields (lines, disabledOverviewLabels). Any callers/tests doing strict equality against the previous { id, name, enabled } shape will break; update the corresponding consumers/tests to include the new properties (or assert via expect.objectContaining).
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@mitchcapper thanks for the PR, but I doubt on this one. Is it something you really need? What should happen when you check something that disappears later? What should happen when you uncheck all? Does it even make sense? All these edge cases make me thing that this kind of customization will bring much more complexity then being usefull. |
|
I am not sure about other people but I check my usage pretty often this consolidating it into one place is great. The problem becomes when you have many providers in use at the same time. Not only is there visual clutter showing models users may not care about (ie flash or chat) but it also means you have to scroll to see what could otherwise be presented on all one screen. "What should happen when you check something that disappears later": Everything is checked by default, unchecking a model adds it to a "hide" list. This also means new models show up by default. In the model goes away it disappears for everyone so the fact you had it checked would not seem to matter. "What should happen when you uncheck all?" Nothing would show. It would kind of be like asking what would happen if you removed all the items from the start menu or remove all your items from your favorites list, it would be empty. Should you ever uncheck all of a specific provider? Probably not i would just move them to the bottom, although I could see some people preferring the cleaner look of them not on the main page at all potentially. It would be trivial to make it recheck everything if you uncheck it all, or you could not uncheck the last item, but again I am not really sure that is a condition one needs to account for. This is a high/frequent touch tool the more streamlined and showing just what the user wants the better I figure. If the feature isn't needed its no problem, I can maintain my own patch (or see about just having it merged downstream). I just try to contribute back things that seem useful to others. If you want to scope down the footprint one could make this a json only feature so no UI for editing, if your concern is UI clutter one could have a json flag controlling if it was enabled or not. |
Description
This allows you to exclude certain models from the main tab in order to help make it more concise. You can still see all models on the individual provider's tab.
Related Issue
Type of Change
Testing
bun run buildand it succeededbun run testand all tests passbun tauri devI am running windows so could only test with the https://github.com/Rana-Faraz/usage-tray-windows fork
Screenshots
The main with filtered models:
The settings to pick what models (only shows the model selection when the provider is enabled):
Checklist
mainbranchSummary by cubic
Add per-provider controls to hide specific model lines on the Overview tab, making the main tab more concise. Settings are saved and do not affect provider-specific tabs.
scope: "overview"line to include/exclude them on Overview.disabledOverviewLabelsto plugin settings with persistence, normalization, and equality checks.ProviderCard; provider tabs remain unchanged. Wiring added viaonToggleOverviewLabelthroughApp,AppContent, and hooks.Written for commit 7361b98. Summary will update on new commits.