fix: keep sidebar filter when opening a table from the filtered list#1707
fix: keep sidebar filter when opening a table from the filtered list#1707mvanhorn wants to merge 2 commits into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
All contributors have signed the CLA ✍️ ✅ |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
|
I have read the CLA Document and I hereby sign the CLA. |
|
Hi @mvanhorn, could you please describe more detail on the behavior both prior to and following this adjustment? |
|
The failing Happy to open a separate PR disambiguating those comparisons (e.g. |
Summary
In the tables sidebar, typing a filter and then double-clicking a table to open it cleared the active filter text. The table opened but the filter was wiped.
Root cause is in
SidebarContainerViewController. TheNSSearchFieldDelegate.searchFieldDidEndSearching(_:)callback unconditionally wrote""back to the persisted search text. When a table is opened, focus leaves the search field, AppKit firessearchFieldDidEndSearching, and the filter was cleared even though the field still held text.The fix guards that callback so it only clears the persisted text when the field's
stringValueis actually empty (the cancel/clear path leaves an empty value). The live-typing path (controlTextDidChange) is untouched, so typing and clearing via the cancel button still work, and tab scoping between the tables and favorites filters is preserved.Why this matters
The maintainer confirmed in #1690 that the filter pane is scoped per table tab and is expected to persist. Prefix filtering (for example
dp_) should survive opening a table; clearing it on every open made the feature unusable for its main purpose.Testing
swiftlint lint --strictpasses on the touched files.TableProTests/Views/SidebarSearchPersistenceTests.swiftcovering the decision: keep the filter when the field still has text, clear it when the field is empty.nonisolatedhelper (shouldClearOnEndSearching) so it is unit-testable without standing up AppKit.Fixes #1690