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

Allow selections to wrap around #976

Merged
merged 2 commits into from
Sep 27, 2022
Merged

Allow selections to wrap around #976

merged 2 commits into from
Sep 27, 2022

Conversation

Saalvage
Copy link
Contributor

It honestly surprised me this wasn't standard behavior.

Tests aren't exhaustive but considering there weren't any tests for the state before this should be good enough.

@phil-scott-78
Copy link
Contributor

Besides the issue with the code style, the code itself looks good. But to be honest I'm not 100% sure what I'm seeing when it comes to what this is trying to solve.

This is the behavior I'm seeing with the current release. There something I'm missing?

Animation

To be clear, I'm not saying there isn't room for this to be better. Just trying to track down current behavior vs new based on the code

@Saalvage
Copy link
Contributor Author

Thanks for getting back to me.

I'm sorry that my original PR didn't explain the feature clearly enough, my bad!

Functionally this change allows you to get to the first from the last element quickly and vice-versa, simply by pressing the corresponding arrow key again when at the edge. Basically as an alternative to using the Home/End keys.

AnsiConsole.Prompt(new SelectionPrompt<string>()
    .Title("What's your favorite fruit?")
    .AddChoices("Apple", "Banana", "Orange", "Pear")
    .WrapAround());

demo

The style issues should be fixed as well!

@phil-scott-78
Copy link
Contributor

Oh jeez, my toddlers killed more brain cells than I thought yesterday. It's so obvious this morning 🤣

I'll take a look again tonight and unless @patriksvensson or @nils-a have any differing opinion on the property name I'll get it merged

@nils-a
Copy link
Contributor

nils-a commented Sep 26, 2022

WrapAround is fine for me. 👍

@patriksvensson
Copy link
Contributor

@phil-scott-78 Happy with WrapAround as well. Unless you have a better suggestion that is.

@phil-scott-78 phil-scott-78 merged commit 422012c into spectreconsole:main Sep 27, 2022
@Saalvage Saalvage deleted the feature/selection-wrap-around branch September 27, 2022 03:51
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.

4 participants