Skip to content

Conversation

@yaira2
Copy link
Member

@yaira2 yaira2 commented Jul 21, 2025

Resolved / Related Issues

To prevent extra work, all changes to the Files codebase must link to an approved issue marked as Ready to build. Please insert the issue number following the hashtag with the issue number that this Pull Request resolves.

Steps used to test these changes

Stability is a top priority for Files and all changes are required to go through testing before being merged into the repo. Please include a list of steps that you used to test this PR.

@yaira2 yaira2 requested review from 0x5bfa and Copilot and removed request for 0x5bfa July 21, 2025 04:44
@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Jul 21, 2025
Copy link
Contributor

Copilot AI left a comment

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 removes legacy address bar and search functionality to simplify the codebase by eliminating the old dual-mode address bar system in favor of the newer Omnibar design. The changes comprehensively remove deprecated search box components, path breadcrumb controls, and their associated view models while cleaning up related UI states and settings.

  • Removes entire legacy search box and path breadcrumb implementation
  • Eliminates dual-mode address bar system (legacy vs Omnibar) in favor of Omnibar-only approach
  • Cleans up obsolete settings, event handlers, and UI state management code

Reviewed Changes

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

Show a summary per file
File Description
ModernShellPage.xaml.cs Removes search box state management and toolbar edit mode checks
ColumnShellPage.xaml.cs Removes search box state management and toolbar edit mode checks
BaseShellPage.cs Removes search box event handlers and edit mode functionality
AdvancedPage.xaml Removes Omnibar enable/disable toggle setting
MainPage.xaml.cs Updates control reference from PathBreadcrumb to Omnibar
MainPage.xaml Removes search box visibility controls and adaptive triggers
Layout pages Removes edit mode checks from keyboard event handlers
SearchBoxViewModel.cs Deletes entire obsolete search box view model
NavigationToolbarViewModel.cs Removes legacy search and edit mode properties and methods
UserControls Deletes SearchBox and PathBreadcrumb controls entirely
Settings/Actions Removes search box related properties and edit mode checks

Copy link
Member Author

Choose a reason for hiding this comment

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

This reverts my earlier fix for a different bug

When using Command mode, switching tabs and then switching back reopens the Omnibar flyout

var command = Commands[hotKey];

if (command.Code is CommandCodes.OpenItem && source?.FindAscendantOrSelf<PathBreadcrumb>() is not null)
if (command.Code is CommandCodes.OpenItem && (source?.FindAscendantOrSelf<Omnibar>() is not null || source?.FindAscendantOrSelf<AppBarButton>() is not null))
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixes the following issue

If a file is selected, focusing on Command mode button via tab navigation and pressing enter opens the selected file

Copy link
Member Author

Choose a reason for hiding this comment

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

@0x5bfa fyi

// Ctrl + V, Paste
case (true, false, false, true, VirtualKey.V):
if (!ToolbarViewModel.IsEditModeEnabled && !ContentPage.IsRenamingItem && !InstanceViewModel.IsPageTypeSearchResults && !ToolbarViewModel.SearchHasFocus)
if (!ContentPage.IsRenamingItem && !InstanceViewModel.IsPageTypeSearchResults)
Copy link
Member Author

Choose a reason for hiding this comment

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


if (!value && SlimContentPage is not ColumnsLayoutPage)
ToolbarViewModel.IsEditModeEnabled = false;

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a way we can force close the Omnibar? This would allow us to solve the issue where its sometimes open after switching tabs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

As of now, theres no method for it. But it looks like we should create one:

// @Omnibar.cs
public void UnfocusOmnibar()
{
    RevertTextToUserInput();
    _textBoxSuggestionsPopup.IsOpen = false;
    _previouslyFocusedElement.TryGetTarget(out var previouslyFocusedElement);
    previouslyFocusedElement?.Focus(FocusState.Programmatic);
}

@yaira2 yaira2 force-pushed the ya/RemovePathBar branch from 0113aa9 to ec01106 Compare July 21, 2025 15:30
Copy link
Member

@0x5bfa 0x5bfa left a comment

Choose a reason for hiding this comment

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

LGTM, code wise.

@yaira2 yaira2 merged commit 6b3b112 into main Jul 21, 2025
8 checks passed
@yaira2 yaira2 deleted the ya/RemovePathBar branch July 21, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Pull requests that are ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants