Opened 8 years ago

Closed 8 years ago

#3144 closed enhancement (implemented)

Network panel : Improve search in the routers list

Reported by: tvataire Owned by: tvataire
Priority: Medium Milestone: Vidalia: 0.3.x
Component: Archived/Vidalia Version:
Severity: Keywords:
Cc: vatairethibault@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Allow to find a router by typing first letters of its nickname.

When a router is selected by this way, allow to select previous or following router by pressing Key_Up or Key_Down.

Child Tickets

Attachments (4)

patch (12.1 KB) - added by tvataire 8 years ago.
Proposed patch to improve router list usability.
search_lineEdit.patch (10.6 KB) - added by tvataire 8 years ago.
Improves search of a router in the routers list by adding a search field.
progressive_search.patch (7.9 KB) - added by tvataire 8 years ago.
0001-Improve-search-in-the-routers-list-by-adding-a-searc.patch (8.3 KB) - added by tvataire 8 years ago.

Download all attachments as: .zip

Change History (15)

Changed 8 years ago by tvataire

Attachment: patch added

Proposed patch to improve router list usability.

comment:1 Changed 8 years ago by tvataire

Status: newneeds_review

comment:2 Changed 8 years ago by tvataire

Cc: vatairethibault@… added

Changed 8 years ago by tvataire

Attachment: search_lineEdit.patch added

Improves search of a router in the routers list by adding a search field.

comment:3 Changed 8 years ago by tvataire

Owner: changed from chiiph to tvataire
Status: needs_reviewassigned

The patch search_lineEdit.patch provides an other way to search a router in the routers list :

It adds a search field above router list, allowing to search a router by it's nickname.

First, search is made for routers whose name match exactly the string of the search field.
If no match is found, search is made for routers whose name start with the string of the search field.

Searches are case-insensitive.

Searches are triggered by pressing Key_Enter or Key_Return.

comment:4 in reply to:  3 ; Changed 8 years ago by chiiph

Replying to tvataire:

The patch search_lineEdit.patch provides an other way to search a router in the routers list :

It adds a search field above router list, allowing to search a router by it's nickname.

First, search is made for routers whose name match exactly the string of the search field.
If no match is found, search is made for routers whose name start with the string of the search field.

I think it's better if the search is always with Qt::MatchStartsWith. It would simplify onRouterSearch() a bit, and since you don't actually filter the router list, I think it will lead to the same behavior you have right now.

Searches are case-insensitive.

Searches are triggered by pressing Key_Enter or Key_Return.

Another possibility that might make this code even cleaner would be to use the signal:

void QLineEdit::textChanged ( const QString & text ) [signal]

in RouterListWidget and you'll have a progresive search.

And yet another possibility would be to use:

void QLineEdit::returnPressed () [signal]

which would lead to the same behavior you have in the patch, with less code.
(The signals would be connected to RouterListWidget::onRouterSearch(QString routerNickname), although depending on the signal used you'd have to change the parameters)

comment:5 in reply to:  4 ; Changed 8 years ago by tvataire

Replying to chiiph:

Replying to tvataire:

The patch search_lineEdit.patch provides an other way to search a router in the routers list :

It adds a search field above router list, allowing to search a router by it's nickname.

First, search is made for routers whose name match exactly the string of the search field.
If no match is found, search is made for routers whose name start with the string of the search field.

I think it's better if the search is always with Qt::MatchStartsWith. It would simplify onRouterSearch() a bit, and since you don't actually filter the router list, I think it will lead to the same behavior you have right now.

I'm not sure. Currently routers whose names match exactly the searched string are selected in priority. Qt::MatchStartsWith is only used as an alternative when Qt::MatchExactly fails.
Always use Qt::MatchStartsWith will select all these routers without distinction, isn't it ?
Do you see a reason to favor this behavior ?

Searches are case-insensitive.

Searches are triggered by pressing Key_Enter or Key_Return.

Another possibility that might make this code even cleaner would be to use the signal:

void QLineEdit::textChanged ( const QString & text ) [signal]

in RouterListWidget and you'll have a progresive search.

And yet another possibility would be to use:

void QLineEdit::returnPressed () [signal]

which would lead to the same behavior you have in the patch, with less code.
(The signals would be connected to RouterListWidget::onRouterSearch(QString routerNickname), although depending on the signal used you'd have to change the parameters)

Progressive search would be a good feature.

Here is what I propose :

  • implement progressive search
  • allow to find next matching item by pressing return key (if several routers have the same names or if their names start with the same pattern)

I didn't used the signal QLineEdit::returnPressed() because it doesn't allow to transmit the name of the router to search for.
The only way I see to use it is :

  • to connect the NetViewer class to this signal
  • when the signal is emited, the NetViewer class retrieve QLineEdit::text(), then calls RouterListWidget::onRouterSearch(QString routerNickname)

Do you think this solution is better ? Is there an other way that I missed ?

comment:6 in reply to:  5 ; Changed 8 years ago by chiiph

Replying to tvataire:

Replying to chiiph:

Replying to tvataire:
I think it's better if the search is always with Qt::MatchStartsWith. It would simplify onRouterSearch() a bit, and since you don't actually filter the router list, I think it will lead to the same behavior you have right now.

I'm not sure. Currently routers whose names match exactly the searched string are selected in priority. Qt::MatchStartsWith is only used as an alternative when Qt::MatchExactly fails.
Always use Qt::MatchStartsWith will select all these routers without distinction, isn't it ?
Do you see a reason to favor this behavior ?

Routers currently are sorted by bandwidth by default, you can change that order by clicking in the columns. I assume that when you search with MatchStartsWith, it will give "priority" to the first match in the current order, right? This seems like a nice "feature", you can change how it searches by just clicking in the columns and changing the order.

Either way, I think that if you have something like this:

name1
name
name2

you type "name", and it selects the second router, and then the first one, and then the third one, it's a really odd behavior, don't you think?
Anyway, I think that up to this point we both understand what we think about the usability part of this feature :)

Progressive search would be a good feature.

Here is what I propose :

  • implement progressive search
  • allow to find next matching item by pressing return key (if several routers have the same names or if their names start with the same pattern)

Yes, that's a nice idea.

I didn't used the signal QLineEdit::returnPressed() because it doesn't allow to transmit the name of the router to search for.
The only way I see to use it is :

  • to connect the NetViewer class to this signal
  • when the signal is emited, the NetViewer class retrieve QLineEdit::text(), then calls RouterListWidget::onRouterSearch(QString routerNickname)

You could just change onRouterSearch to RouterListWidget::onRouterSearch(), then connect QLineEdit::returnPressed() to it, and on the first line of onRouterSearch you could do:

QString routerNickname = somelineedit.text();

and the rest of the slot stays the same. You hook up this same slot to returnPressed and textChanged to support progressive search. I'm not sure if you connect it to textChanged only, what would happen when you press return. Or you can just do what you said :)

comment:7 in reply to:  6 Changed 8 years ago by tvataire

Replying to chiiph:

Replying to tvataire:

Replying to chiiph:

Replying to tvataire:
I think it's better if the search is always with Qt::MatchStartsWith. It would simplify onRouterSearch() a bit, and since you don't actually filter the router list, I think it will lead to the same behavior you have right now.

I'm not sure. Currently routers whose names match exactly the searched string are selected in priority. Qt::MatchStartsWith is only used as an alternative when Qt::MatchExactly fails.
Always use Qt::MatchStartsWith will select all these routers without distinction, isn't it ?
Do you see a reason to favor this behavior ?

Routers currently are sorted by bandwidth by default, you can change that order by clicking in the columns. I assume that when you search with MatchStartsWith, it will give "priority" to the first match in the current order, right? This seems like a nice "feature", you can change how it searches by just clicking in the columns and changing the order.

Either way, I think that if you have something like this:

name1
name
name2

you type "name", and it selects the second router, and then the first one, and then the third one, it's a really odd behavior, don't you think?
Anyway, I think that up to this point we both understand what we think about the usability part of this feature :)

ok, I better understand your point of view.
I chosen this behavior to avoid to have to sort the list by name before doing a search, but I agree, this isn't intuitive.

Progressive search would be a good feature.

Here is what I propose :

  • implement progressive search
  • allow to find next matching item by pressing return key (if several routers have the same names or if their names start with the same pattern)

Yes, that's a nice idea.

I didn't used the signal QLineEdit::returnPressed() because it doesn't allow to transmit the name of the router to search for.
The only way I see to use it is :

  • to connect the NetViewer class to this signal
  • when the signal is emited, the NetViewer class retrieve QLineEdit::text(), then calls RouterListWidget::onRouterSearch(QString routerNickname)

You could just change onRouterSearch to RouterListWidget::onRouterSearch(), then connect QLineEdit::returnPressed() to it, and on the first line of onRouterSearch you could do:

QString routerNickname = somelineedit.text();

and the rest of the slot stays the same.

ok I'll try this.

You hook up this same slot to returnPressed and textChanged to support progressive search. I'm not sure if you connect it to textChanged only, what would happen when you press return. Or you can just do what you said :)

Changed 8 years ago by tvataire

Attachment: progressive_search.patch added

comment:8 Changed 8 years ago by tvataire

progressive_search.patch :

  • add progressive search
  • use returnPressed() signal instead of keyPressEvent(QKeyEvent * ) function to detect Key_Return and Key_Enter events in the search field
  • always use Qt::MatchStartsWith to search routers in the list

comment:9 Changed 8 years ago by chiiph

Milestone: Vidalia-0.3.X

Sorry for the delay.

The patch looks good :)

The last details: I want this to be in alpha rather than master, since I'm using master as a bugfix branch. Could you port your patch to that branch? Also, you should look into git format-patch so you can be the author of the commit.

comment:10 Changed 8 years ago by tvataire

Summary: Network panel : Improve router list usabilityNetwork panel : Improve search in the routers list

Great ! I was waiting a long time for this feature.

And thanks for your help and advice. :)

comment:11 Changed 8 years ago by chiiph

Resolution: implemented
Status: assignedclosed

Ok, after all this waiting, this finally got merged. So sorry for the delay.

Feel free to work on any other patches/tickets :)
Thanks!

Note: See TracTickets for help on using tickets.