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.

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

FEAT(client): Search dialog #4967

Merged
merged 7 commits into from May 11, 2021
Merged

Conversation

Krzmbrzl
Copy link
Member

@Krzmbrzl Krzmbrzl commented May 5, 2021

This PR introduces a fully featured search dialog.

Note that selections in the search dialog are synchronized with the main
UI and thus "whisper to selection" works by selecting something in the
search dialog as well.

There are also default actions that are executed when a search result is
activated (double-clicked or pressing enter on it). What that action is
exactly, can be configured in the settings.

Furthermore the context menu works as expected when invoked on entries
in the search result list.

In order to disambiguate the results, the full channel hierarchy to the
search result's parent channel is shown below each result.

The search dialog can be toggled via Ctrl+F when Mumble has focus or by
using the new entry in the toolbar. Additionally there is a new global
shortcut that can be configured for this purpose. Note however that on
Windows the SearchDialog won't obtain focus when toggled in a situation
when Mumble does not have focus already.

Checks

image


Note: The previous "search as you type" functionality got replaced by this new functionality and is therefore no longer available

@Krzmbrzl Krzmbrzl added client ui feature-request This issue or PR deals with a new feature labels May 5, 2021
@Krzmbrzl Krzmbrzl changed the title REFAC(client): Move utility functions to own file FEAT(client): Search dialog May 5, 2021
@vimpostor
Copy link
Contributor

vimpostor commented May 7, 2021

This is a very good idea, I like it a lot!
I am not sure about the UI of the 4 checkboxes on the bottom though. Clearly the two first boxes are related: Users and Channels specify what the search results can be. And then the last two checkboxes are related: Case sensitivity and regex specify the way that the search is done.

I think UI-wise it would be much better for the user to have them grouped in those two categories.
There would be multiple ways to do that: Leaving more space in the middle would already be nice, but you could also group them together in other ways...

@vimpostor
Copy link
Contributor

vimpostor commented May 7, 2021

Perhaps another good solution would be two rows:

  • The first one has a label called something like "Search for:" followed by the first two checkboxes
  • The second one has a label called something like "Search options" followed by the last two checkboxes

I think especially the label for the first row would make it much clearer for the user what these do.

I take it that the settings icon on the right unhides the options? Then it wouldn't be a problem space-wise to have two rows anyway.

@Krzmbrzl
Copy link
Member Author

Krzmbrzl commented May 7, 2021

I take it that the settings icon on the right unhides the options? Then it wouldn't be a problem space-wise to have two rows anyway.

Yes indeed.

Thanks for the feedback. I have implemented your second suggestion and I'll update the screenshot accordingly :)

Krzmbrzl and others added 5 commits May 11, 2021 08:58
Instead of implicitly assuming that the root channel has ID 0, there is
now an explicit constant (of the same value) that is used in all places
instead of literal "0" when referring to the root channel's ID.
This setting will be needed for other purposes as well and therefore
putting this into the TalkingUI group of settings in no longer
applicable.
Before the channel tree (UserView) in the main UI had a "feature" called
keyboard search. If the channel tree had focus and the user started
typing on their keyboard, Mumble would search for an entry in the tree
(either channel or user) that matches the invisibly typed search String.

This is however very impractical for searching as it is unclear if and
when this will expand collapsed entries or how subsequent searches work.
It is not even clear when the current search has ended and thus typing
another letter will start a new one.

And for users who don't expect such an invisible feature, this is a bad
experience as their channel tree selection jumps around wildly if they
accidentally push a key.

The search functionality will be provided by a more robust
implementation in a following commit.
The shortcut for toggling channel filters was Ctrl+F. While this makes
sense semantically, this specific shortcut is associated with starting a
search in 99% of all applications out there. Thus using it for something
else does not make for a very smooth user experience.

The new shortcut for filtering is thus set to be Alt+F instead.
This commit introduces a fully featured search dialog.

Note that selections in the search dialog are synchronized with the main
UI and thus "whisper to selection" works by selecting something in the
search dialog as well.

There are also default actions that are executed when a search result is
activated (double-clicked or pressing enter on it). What that action is
exactly, can be configured in the settings.

Furthermore the context menu works as expected when invoked on entries
in the search result list.

In order to disambiguate the results, the full channel hierarchy to the
search result's parent channel is shown below each result.

The search dialog can be toggled via Ctrl+F when Mumble has focus or by
using the new entry in the toolbar. Additionally there is a new global
shortcut that can be configured for this purpose. Note however that on
Windows the search dialog won't obtain focus when toggled in a situation
when Mumble does not have focus already.
@Krzmbrzl Krzmbrzl merged commit 8c394b1 into mumble-voip:master May 11, 2021
@Krzmbrzl Krzmbrzl deleted the feat-search branch May 11, 2021 11:18
@deluxghost
Copy link
Contributor

there is a string on weblate: Whether to search for clients

you mean channels?

@Krzmbrzl
Copy link
Member Author

@deluxghost There should be both. Once for clients and once for channels

@Krzmbrzl
Copy link
Member Author

Ah no. I see what you mean. Yeah that's an error. Thanks for pointing this out 👍

@Krzmbrzl
Copy link
Member Author

See #4985

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client feature-request This issue or PR deals with a new feature ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants