Skip to content

Conversation

@fgemig
Copy link

@fgemig fgemig commented Oct 6, 2020

Summary

The proposed change makes it possible to change the color in the selected ListInput item.

Motivation

This change improves the way the user views the selected items in the list, avoiding choosing an incorrect item.

Example

exemplo-inquirer

Well, I hope that this implementation will be useful for the community. I am open to questions and accept feedbacks!

@fgemig fgemig changed the title Refactoring the ListInput method to change the color of the selected … Implementing color change in the selected item from ListInput … Oct 6, 2020
@fgemig fgemig changed the title Implementing color change in the selected item from ListInput … Implementing color change in the selected item from ListInput Oct 6, 2020
Copy link
Owner

@afucher afucher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @fgemig , sorry for the delay to check you PR.
I made some comments, could you check please?

Thanks for contributing!

console.WriteLine(item.Message);
});
Newline();
console.ResetColor();
Copy link
Owner

@afucher afucher Oct 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this console.ResetColor() should be inside the loop.
If we have 3 messages:

  1. Console.Color = Cyan
  2. Console.Color = (Not set)
  3. Console.Color = Red

The second message will print Cyan, and I was expecting it to be the default Console color.

Could you please add some test to cover that? If need any help let me know

});
Newline();
console.ResetColor();
bottomContent.ToList().ForEach(item => console.WriteLine(item.Message));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the bottomContent is also of the type ConsoleMessage, we should check the Color the same way that you have done for the content.

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.

2 participants