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

Add option to configure the style of the choices list and the default value in TextPrompt #681

Merged
merged 4 commits into from
Mar 27, 2022

Conversation

ap0llo
Copy link
Contributor

@ap0llo ap0llo commented Jan 6, 2022

TextPrompt currently uses a fixed style for the default value (green text) and the list of choices (blue text).

This PR adds two new properties to TextPrompt (DefaultValueStyle and ChoicesStyle) that allow customozing the style of these values.
If no styles are set, the current green & blue values continue to be used.

Note: I'm unsure if there are tests that cover the styling of TextPrompt. If there is a place I can add a test for this, it would be great if you could point me in the right direction.

@CLAassistant
Copy link

CLAassistant commented Jan 6, 2022

CLA assistant check
All committers have signed the CLA.

@nils-a
Copy link
Contributor

nils-a commented Mar 22, 2022

@ap0llo sorry for the late reply.
The change looks good.

As to the tests: There are currently no tests for styling of the TextPrompt. I would place the new tests in ./test/Spectre.Console.Tests/Unit/Prompts/TextPromptTests.cs

Add property "DefaultValueStyle" to TextPrompt which allows setting the style in which the default value is rendered.
When no style is set, the value is displayed as green text, keeping the existing behavior.
Add property "ChoicesStyle" to TextPrompt which allows setting the style in the prompt's choices are rendered.
When no style is set, choices are displayed as blue text, keeping the existing behavior.
@ap0llo
Copy link
Contributor Author

ap0llo commented Mar 27, 2022

@nils-a I added some simple unit tests to ensure the styles are applied

Copy link
Contributor

@nils-a nils-a left a comment

Choose a reason for hiding this comment

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

LGTM!

@nils-a nils-a added this to the 0.44 milestone Mar 27, 2022
@nils-a nils-a merged commit 38d9386 into spectreconsole:main Mar 27, 2022
@nils-a
Copy link
Contributor

nils-a commented Mar 27, 2022

@ap0llo your changes have been merged, thanks for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done 🚀
Development

Successfully merging this pull request may close these issues.

3 participants