Skip to content

RadioList: search #758

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

Closed
wants to merge 2 commits into from
Closed

RadioList: search #758

wants to merge 2 commits into from

Conversation

gpotter2
Copy link
Contributor

This PR genually improves RadioList by:

  • binding the escapekey to cancel
  • adding support for the pageup and pagedown keys
  • adds a search system, demonstrated below

Prompt_toolkit RadioList.search v1

I'm of course open to any suggestions.

Note: I tried several implementations of the searching system. The first one aimed at adding > [the text] above or below the frame. However, due to how the scrolling is implemented, it was harder than expected, which is why I rolled-back to a simpler color-based searching system. I can elaborate if needed

@jonathanslenders
Copy link
Member

Hi @gpotter2,
Thanks a lot for this proposal!
There are some good things in here. The page up/down is definitely something I'll merge.
For the rest I need some time to think about it.
I hope that's fine for you!

@gpotter2
Copy link
Contributor Author

gpotter2 commented Oct 24, 2018

Hi,
Would you like me to split the PR ?

I understand that the search system I a bit « raw » and would require some tweaks

@jonathanslenders
Copy link
Member

Hi @gpotter2,
That would be nice. If you can create a separate PR with only the page up/down changes, I can merge that already.

Notice that actually for a text area, page up/down also changes the scroll offset of the window. Pressing page-down keeps the cursor at the top of the window, and page-up keeps it at the bottom of the window. For now, I propose however to keep it like it is.

@codecov-io
Copy link

Codecov Report

Merging #758 into master will decrease coverage by 0.13%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #758      +/-   ##
==========================================
- Coverage   70.99%   70.86%   -0.14%     
==========================================
  Files         144      144              
  Lines       13616    13642      +26     
==========================================
  Hits         9667     9667              
- Misses       3949     3975      +26
Impacted Files Coverage Δ
prompt_toolkit/shortcuts/dialogs.py 28.57% <0%> (-0.38%) ⬇️
prompt_toolkit/widgets/base.py 27.14% <0%> (-2.67%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aae62c8...f32f08a. Read the comment docs.

@gpotter2 gpotter2 changed the title RadioList: pagedown/pageup support + search RadioList: search Oct 24, 2018
@gpotter2
Copy link
Contributor Author

@jonathanslenders Here it is #761
I've also reverted it from this PR.

Please tell me if you have in mind an implementation of a searching system that would fit better.

@gpotter2
Copy link
Contributor Author

gpotter2 commented Feb 9, 2019

Hey ! Sorry for the ping, any news on such a thing ?

I understand you have little time for this, feel free to tell me any improvements/changes that should be done, or that you don't want to implement it that way for X reasons 😄

@gpotter2
Copy link
Contributor Author

gpotter2 commented May 2, 2019

Closed for inactivity, and because it's going to conflict with too many things soon.

This could probably be integrated in a much more elegant way (using some internal functions / bindings to the existing search API ?)

@gpotter2 gpotter2 closed this May 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants