Support trailing widgets on settings page title and Category headers (APP-5559) - #15345
Draft
warp-agent-staging[bot] wants to merge 3 commits into
Draft
Support trailing widgets on settings page title and Category headers (APP-5559)#15345warp-agent-staging[bot] wants to merge 3 commits into
warp-agent-staging[bot] wants to merge 3 commits into
Conversation
…PP-5559) Adds an optional trailing widget accessory to the PageType title slot (Uncategorized/Categorized) and to Category headers, modeled as Box<dyn SettingsWidget> so they carry search_terms and stay visible to settings search. Search semantics: - The page-title trailing widget follows the title, which already renders unconditionally on these variants. - A category's trailing widget shows when the category survives filtering because a child widget matched, or because the trailing widget's own search_terms match the query. Categories with no surviving widgets (children or trailing) are still dropped. This is framework-only groundwork for porting the Warp Agent settings page to PageType::Categorized; no existing page's behavior changes.
Contributor
Author
|
This PR was generated with Warp. Comment |
Address review findings on PR #15345: - PageType::with_title_trailing_widget now rejects Uncategorized/ Categorized pages built with no title (in addition to the existing Monolith rejection), leaving the accessory unset after a debug assertion instead of letting it count toward search matches while never rendering. - Category::with_trailing_widget now rejects the empty-title "no header" category shape (e.g. Warpify's) for the same reason. - Both builders now document the API contract that accessories must be compact trailing controls only, not page titles, section headers, or body rows. - Replaced the two title-trailing tests that only checked MatchData::is_truthy() with tests that pattern-match the exact MatchData::Countable(1) and destructure FilteredPageType to assert the accessory is still Some, for both a title-only and a child-only query. - Added #[should_panic] regression tests proving both builders reject their headerless combination.
The two #[should_panic] regression tests for the headerless compiled out under --release, so both tests silently stopped panicking and failed their #[should_panic] expectation there. Gate each behind #[cfg(debug_assertions)] and pair it with a #[cfg(not(debug_assertions))] test that exercises the same contract without relying on the panic: build the invalid combination, then assert via update_filter/get_filtered that the accessory is left unset and contributes nothing to MatchData. The rejection itself stays debug-only, matching the intent that this guards a caller-programming-error, not an end-user-facing runtime condition.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Description
Framework-only groundwork for APP-5559: porting the Warp Agent settings page to
PageType::Categorizedis blocked by two accessories that don't fit the current model — a switch next to the page title, and a switch next to one category's header. This PR (1 of 3) adds framework support for both, without touching any existing page's behavior or porting Warp Agent (that's PR 2) or splitting its mega-widget (PR 3).Adds an optional trailing widget to:
PageTypetitle slot (Uncategorized/Categorized), viaPageType::with_title_trailing_widget.Categoryheaders, viaCategory::with_trailing_widget.Both are modeled as
Box<dyn SettingsWidget>(not a bareElement) so they carrysearch_termsand stay visible to settings search, and are threaded throughFilteredPageType/FilteredCategory,update_filter,get_filtered, andrender_page. The header row layout follows the existing space-betweenFlexrow pattern inActiveAIWidget(app/src/settings_view/warp_agent_page.rs).Search semantics
PageTypevariants today, so its accessory just follows it, and itssearch_termsmatch now also counts toward the page'sMatchDataso a query that only matches the accessory doesn't drop the page from the sidebar.search_termsmatch — so a master switch never disappears just because a child row matched, and a trailing-only match resurfaces the category showing its trailing widget (not nothing).Monolithpages are intentionally not touched — their title visibility is already tied to their sole widget matching, which is a different mechanism than the two widget-list variants this PR addresses.All existing pages are unaffected: the new fields default to
Noneand every current constructor/caller keeps working unchanged.Linked Issue
ready-to-implement.Testing
Added unit tests in
app/src/settings_view/mod_tests.rscovering:MatchData(and not inflating it when it doesn't match).Ran:
cargo test -p warp --lib settings_view::tests— all 44 tests pass (including the new ones).cargo fmt -p warp -- --check— clean.cargo clippy -p warp --lib --tests -- -D warnings— clean.This PR has no user-visible surface (no page currently attaches a trailing widget), so there is nothing to manually test in the running app yet; that lands with the Warp Agent port in PR 2.
./script/runAgent Mode
CHANGELOG-NONE