Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Power Rename] Select all while filter is active #24598

Merged
merged 2 commits into from
Mar 17, 2023
Merged

[Power Rename] Select all while filter is active #24598

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
18b11bf
Check if the result from find item returns a valid item
sosssego Mar 6, 2023
ba412a6
Call all the clean and populate methods only
sosssego Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/modules/powerrename/PowerRenameUILib/MainWindow.xaml.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,13 @@ namespace winrt::PowerRenameUI::implementation
{
ToggleAll();
m_allSelected = !m_allSelected;
if (button_showRenamed().IsChecked())
{
m_explorerItems.Clear();
m_explorerItemsMap.clear();
PopulateExplorerItems();
UpdateCounts();
}
}
}

Expand Down Expand Up @@ -738,7 +745,8 @@ namespace winrt::PowerRenameUI::implementation
int id = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

even better, instead of iterating here over all elements (GetItemCount), you can iterate over m_explorerItemsMap (or m_explorerItems), and check/unchech those items, as these collections are updated on ShowAll/ShowRenamed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current ToggleAll function changes the PowerRenameItem selected and the ExplorerItem selected so I thought it would make it more complex to run the loop twice.

Copy link
Collaborator

Choose a reason for hiding this comment

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

not running the loop twice. Just loop through m_explorerItemsMap and then you can get PowerRenameItem with PRManger->GetById and change it that way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It also needs to go through the ones in m_prManager that are not in m_explorerItemsMap to change its select state.

Copy link
Collaborator

@stefansjfw stefansjfw Mar 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

There are 2 options here when showing only the files that will be renamed and clicking Select All/Unselect All:

  1. Select All/Unselect All for all items that are loaded into PowerRename
  2. Select All/Unselect All only the items that can be renamed

Current behavior is 1 I think, right? But I'd prefer to go with 2, as it makes more sense from UX side. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it selects/unselects everything loaded in powerRename like in option 1.
for (auto& [key, value] : m_explorerItemsMap) { value.Checked(selected); }
https://user-images.githubusercontent.com/14083829/225715805-85d1c2a0-5415-467b-bc85-fa66acecb4c0.mp4

Option 2 may allow unchecking only what is being renamed, and selecting and unselecting it.
for (UINT i = 0; i < itemCount; i++) { CComPtr<IPowerRenameItem> spItem; if (SUCCEEDED(m_prManager->GetItemByIndex(i, &spItem))) { spItem->PutSelected(selected); int id = 0; spItem->GetId(&id); auto item = FindById(id); if (item) item.Checked(selected); } }
https://user-images.githubusercontent.com/14083829/225715823-7fc157a5-bbc8-49fe-a4f2-ed7f27e1505e.mp4

But sometimes It feels not intuitive.
I still prefer option 1, check the vídeos.

spItem->GetId(&id);
auto item = FindById(id);
item.Checked(selected);
if (item)
item.Checked(selected);
}
}
UpdateCounts();
Expand Down