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

Implemented #1034 #1037

Closed
wants to merge 3 commits into from
Closed

Implemented #1034 #1037

wants to merge 3 commits into from

Conversation

oscargus
Copy link
Contributor

Added a button that can be used to determine if the search results should be updated on every pressed key or on enter only. Based on the suggestion in #1034

Two issues:

  • Select a good icon. The current one is quite intuitive if it wasn't for the fact that the symbol is rarely used for selecting a state.
  • When using autocompleting and search on enter, enter needs to be pressed twice, once to select the completing, once to search

searchontype

Builds will be available at http://builds.jabref.org/livesearch in 20 minutes or so.

  • Change in CHANGELOG.md described
  • Screenshots added (for bigger UI changes)

@oscargus oscargus added type: enhancement ui status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers labels Mar 25, 2016
@tobiasdiez
Copy link
Member

I don't like the boolean parameter to much. I would prefer if the performSearch method is not changed but a second method performLiveSearch (including the check) is introduced. Otherwise good job.

@oscargus
Copy link
Contributor Author

I do not know why I so rarely use overloading (which this is a sort of)... I changed now. performSearch() and performSearchIfLive()

@oscargus
Copy link
Contributor Author

Changed to the "sync" icon. Not sure why Eclipse formats the IconTheme file as it does, but I cannot get around it...

newsearch

@oscargus
Copy link
Contributor Author

Found auto-renew so changed to that. Basically a mirrored version of sync, so clockwise arrows.

@simonharrer
Copy link
Contributor

Hm, idea: what about issuing the search request only after 500ms of nothing happened in the text field? This would require less icons and less buttons the user has to interact and understand, and could provide similar results. what do you think @oscargus

@oscargus
Copy link
Contributor Author

I see your point regarding the number of buttons, but then it seems like one either has a small enough database and then it is probably just annoying or you have a big database in which case it might still not solve the issue.

It seems like this setting and filter/float are user preferences rather than search preferences (in the sense that you won't change this depending on your current search, as opposed to regex and case). Maybe one should tuck the user preferences away as they are more rarely changed. I mean, you would like to have live search by default but be able to disable it if required. I can also imagine that some people want to get rid of autocompletion sometimes (with that said I haven't checked if one can configure it). An "advanced settings" button with pop-up menu?

@simonharrer
Copy link
Contributor

I am somewhat unsure where we should be heading here. I think we should try to reduce the number of preferences the user can input as JabRef has so many options, it can make it very hard to use.

If we detect whether the database is small, medium or large in size, we could adjust the search automatically somehow? So it will not update immediatly if there are more than 1k entries? Or use a formula which computes the amount to wait between after a search. The issue is not the search itself (which is done in a separate thread) but the update of the UI after the search is done.

So basically, we could try to keep track of the search queries that are currently active through an atomic counter. And only update the UI when the counter has reached the value 1.

@oscargus
Copy link
Contributor Author

I agree on the preference thing in general. Still I think this is a rather straightforward setting and more importantly it doesn't really break anything if set the "wrong" way.

Somehow, these automatic things will always turn out bad for some users and then they cannot change it. Apart from that I agree that it should be possible to come up with a scheme that is reasonable for most users. However, I'm not convinced it is worth the effort compared to the simple and fully waterproof switch. :-)

To me it can even be hidden in the preferences under a "performance" tab, where that and e.g. the new cell formatting can be switches off if required. All bells and whistles on, but can be disabled if performance is suffering.

I have some thoughts related to preferences in general and will open an issue any day now.

@simonharrer
Copy link
Contributor

Hm, ok. Maybe the quickest thing is to add a settings dropdown menu using some kind of wrench or so symbol in the search bar. There, we can also move the float thing as well.

@simonharrer
Copy link
Contributor

@oscargus nevertheless, I think we should ensure this overlapping of searches does not happen. This could maybe help enough already, reducing the pain of having a large database without loosing the live feature.

@tobiasdiez
Copy link
Member

I'm with Simon here and think a button is not necessary. Why not just perform a search every time directly after the input changed? If the user types quicker then the search is performed, then the old search is just aborted and a new one started. So for small databases you see a direct consequences, for larger ones you get a delayed response. The only thing required for this approach is to make sure that an ongoing search does not block inputs in the search field.
Google also somehow manages to show live results without a toggle instant search button and I believe their database is bigger than the average bib file 😄

@oscargus
Copy link
Contributor Author

So the user response in #1034 is irrelevant?
Den 29 mar 2016 21:48 skrev "Tobias Diez" notifications@github.com:

I'm with Simon here and think a button is not necessary. Why not just
perform a search every time directly after the input changed? If the user
types quicker then the search is performed, then the old search is just
aborted and a new one started. So for small databases you see a direct
consequences, for larger ones you get a delayed response. The only thing
required for this approach is to make sure that an ongoing search does not
block inputs in the search field.
Google also somehow manages to show live results without an instant search
button and I believe their database is bigger than the average bib file [image:
😄]


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1037 (comment)

@tobiasdiez
Copy link
Member

No of course not, but I find the part blocking further typing into the search box the most critical and annoying. If the input in the search field is not blocked, the user will not notice that we actually search in the background.

@oscargus
Copy link
Contributor Author

Ah, OK! Now I understand your comment. Sure, that would be another way to solve it. Probably better from many perspectives.

@oscargus
Copy link
Contributor Author

oscargus commented Apr 4, 2016

I tried to implement a settings button and while it works I'm not really fond of the results. I've tried having both one menu item for filter/float (which is quite unclear) and having radio buttons for both options (which is better as one can have the full sentence), but I think that the additional icon is still the best. The whole search bar is quite compact now, so I do not think that the previous width problem is relevant. Also, if/when live search is removed (e.g. if non-overlapping search is implemented and works for 100000 entries...) it will be easier to just remove the button rather than remove the menu etc.

@simonharrer
Copy link
Contributor

Do you think this is still necessary or this can be closed in favor having a faster overall search?

@oscargus
Copy link
Contributor Author

oscargus commented Apr 7, 2016

No, if the search speed has improved as much as it seems, I do not think this one is required. (If it turns out to be, it is easy to recover.)

@oscargus oscargus closed this Apr 7, 2016
@stefan-kolb stefan-kolb deleted the livesearch branch April 7, 2016 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers type: enhancement ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants