Added checkbox locked choices feature #327

NivEz · 2022-12-27T00:01:34Z

Checkbox locked choices feature:

This feature gives the option lock some choices.
Locked choices cannot be removed (with space or left button) and they are merged with the default choices, so they appear as checked choices.
This is similar to giving choices to the user which must be chosen in order to continue with the process (like user agreement). The only option not to choose the locked option is to break the program.
This is useful if you want to make it clear that some choice/s out of the list must be chosen in order to continue.

BTW this package is awesome and I use it for installation tool that I might publish soon (created this PR because I needed this feature in my project).

staticdev

@NivEz thanks a lot for your contribution. Please check out tests in tests folder, we are committed to delivering a high quality code and having unit, integration and acceptance tests are a way to ensure that. Also, I see that current code is failing pre-commit checks, you can run it locally with pre-commit install and then pre-commit run --all-files. You can find a bit more details in our contributing guide.

NivEz · 2023-01-01T23:12:33Z

@NivEz thanks a lot for your contribution. Please check out tests in tests folder, we are committed to delivering a high quality code and having unit, integration and acceptance tests are a way to ensure that. Also, I see that current code is failing pre-commit checks, you can run it locally with pre-commit install and then pre-commit run --all-files. You can find a bit more details in our contributing guide.

Hey,
I pushed another commit to fix the pre-commit errors. It seems that the tests passed except for codecov/patch. I am not really sure what is the issue, can you guide me?

Thanks

Cube707 · 2023-01-02T19:21:20Z

The coverage/patch displays the coverage of your added lines, which is currently 83.3%. This means that not all of the lines added by you are coverd by allready existing tests. This is to be expected, as you added branches to the code (new if statements, etc.) but didn't add tests for these cases.

To increase the coverage, simply add the missing tests.

Note
Please check out tests in tests folder, we are committed to delivering a high quality code and having unit, integration and acceptance tests are a way to ensure that.

Cube707 · 2023-01-02T19:31:39Z

for a detailed breakdown, see the codecov webpage. As you can see there, your added if's are marked yellow, as only on one of the possible code branches is covered (the one thats skips the if block is missing).

There is also a existing problem further up, where a line is not covered at all (maked red), you can ignore it but we missed it on a code review which is exacly what this test are here to prevent as much as possible

src/inquirer/render/console/_checkbox.py

Cube707

I think it should be visually more obvious that choises are locked. For example by changing the color (maybe a gray?)

… duplicates removal

NivEz · 2023-01-06T17:50:08Z

I think it should be visually more obvious that choises are locked. For example by changing the color (maybe a gray?)

Good point, added gray color

Cube707

Just a thought: should it really be assumed that locked choise must be selected? wouldn't it also make senese to lock choises as they are and use the default setting to choose if they are selected or not. As it stands now, the functionalaty overlapps and its not possible to display options that are no longer choosable based on previous inputs (I mean you could just remove these options all together, but if we want to lock stuff this is a valid usecase??)

also it would be great to make the commits more atomic and have less small commits that just fix stuff from a previous commit. You can do this by resetting to a previous commit and appending to it, than cherry pick the other commits. You can force-push it to your branch and the PR will update.

This is not mandatory and just a preference of mine to have clean commits that contain all changes belonging to a feature. For example: one commit for the feature, one commit for the tests and one commit for the documentations

src/inquirer/themes.py

Cube707 · 2023-01-08T14:03:57Z

tests/acceptance/test_checkbox.py

+
+    def test_locked_option(self):
+        self.sut.send(key.SPACE)
+        self.sut.send(key.LEFT)


Its a good idea to test for both keys to select a option.

But in this case the test would actually succseed even if the locking code didn't work, as it deselects and than imediatly selects againt. So please seperate this into two seperate tests

Seprated into two tests

Cube707 · 2023-01-08T14:05:18Z

tests/integration/console_render/test_checkbox.py

@@ -324,3 +324,59 @@ def test_double_invert_all(self):
        result = sut.render(question)

        assert result == []
+
+    def test_unselect_locked(self):
+        stdin_array = [key.SPACE, key.LEFT, key.ENTER]


same as above

Seprated into two tests as well

Cube707 · 2023-01-08T14:11:32Z

src/inquirer/render/console/_checkbox.py

        self.current = 0

+    def set_default_choices(self):


this function is named strangely. I assume you wanted to make its return type clear? (which changed in the meantime)

However like this its named as a Setter-Methode, which it is clearly not.

Suggested change

def set_default_choices(self):

def default_choices(self):

Cube707 · 2023-01-08T21:02:01Z

I also just fixed the security issue on main, so please rebase

NivEz · 2023-02-03T19:53:52Z

@Cube707
I added fixes according to the comments you left. Didn't have much time since I was very busy.

Thanks for reviewing :)

no longer maintaining this

Cube707 · 2023-03-06T19:16:09Z

@NivEz sorry it took so long, but I finally had the time to look at this and merge it. Hope it can still be helpfull to you

NivEz added 2 commits December 27, 2022 01:33

added checkbox locked choices feature

39eb6be

added explanation in the docs about locked feature

3a6cee7

NivEz requested review from magmax and staticdev as code owners December 27, 2022 00:01

staticdev previously requested changes Dec 28, 2022

View reviewed changes

fixed pre-commit errors

27c0418

Cube707 reviewed Jan 2, 2023

View reviewed changes

src/inquirer/render/console/_checkbox.py Outdated Show resolved Hide resolved

Cube707 reviewed Jan 2, 2023

View reviewed changes

src/inquirer/render/console/_checkbox.py Outdated Show resolved Hide resolved

Cube707 requested changes Jan 2, 2023

View reviewed changes

NivEz added 3 commits January 6, 2023 02:04

[fixed] removed redundent parantases and cancelled casting to set for…

4161849

… duplicates removal

added locked checkbox test

5ee64bb

added gray color to locked options

bf20f55

Cube707 requested changes Jan 8, 2023

View reviewed changes

NivEz force-pushed the main branch 2 times, most recently from 1935044 to fc055c1 Compare February 3, 2023 19:23

fixed PR comments

5f51882

NivEz force-pushed the main branch from 8dc3f6c to 5f51882 Compare February 3, 2023 19:46

Merge branch 'magmax:main' into main

fcb98ba

NivEz requested review from Cube707 and removed request for magmax February 8, 2023 15:06

Cube707 approved these changes Mar 6, 2023

View reviewed changes

Cube707 added the python Pull requests that update Python code label Mar 6, 2023

Cube707 merged commit 0af3ccb into magmax:main Mar 6, 2023

Cube707 mentioned this pull request Aug 14, 2023

[Question] Does it support to make required input? #435

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added checkbox locked choices feature #327

Added checkbox locked choices feature #327

NivEz commented Dec 27, 2022 •

edited

Loading

staticdev left a comment •

edited

Loading

NivEz commented Jan 1, 2023 •

edited

Loading

Cube707 commented Jan 2, 2023 •

edited

Loading

Cube707 commented Jan 2, 2023 •

edited

Loading

Cube707 left a comment

NivEz commented Jan 6, 2023

Cube707 left a comment •

edited

Loading

Cube707 Jan 8, 2023 •

edited

Loading

NivEz Feb 3, 2023

Cube707 Jan 8, 2023

NivEz Feb 3, 2023

Cube707 Jan 8, 2023 •

edited

Loading

NivEz Feb 3, 2023

Cube707 commented Jan 8, 2023

NivEz commented Feb 3, 2023 •

edited

Loading

Cube707 commented Mar 6, 2023

Added checkbox locked choices feature #327

Added checkbox locked choices feature #327

Conversation

NivEz commented Dec 27, 2022 • edited Loading

Checkbox locked choices feature:

staticdev left a comment • edited Loading

Choose a reason for hiding this comment

NivEz commented Jan 1, 2023 • edited Loading

Cube707 commented Jan 2, 2023 • edited Loading

Cube707 commented Jan 2, 2023 • edited Loading

Cube707 left a comment

Choose a reason for hiding this comment

NivEz commented Jan 6, 2023

Cube707 left a comment • edited Loading

Choose a reason for hiding this comment

Cube707 Jan 8, 2023 • edited Loading

Choose a reason for hiding this comment

NivEz Feb 3, 2023

Choose a reason for hiding this comment

Cube707 Jan 8, 2023

Choose a reason for hiding this comment

NivEz Feb 3, 2023

Choose a reason for hiding this comment

Cube707 Jan 8, 2023 • edited Loading

Choose a reason for hiding this comment

NivEz Feb 3, 2023

Choose a reason for hiding this comment

Cube707 commented Jan 8, 2023

NivEz commented Feb 3, 2023 • edited Loading

Cube707 commented Mar 6, 2023

NivEz commented Dec 27, 2022 •

edited

Loading

staticdev left a comment •

edited

Loading

NivEz commented Jan 1, 2023 •

edited

Loading

Cube707 commented Jan 2, 2023 •

edited

Loading

Cube707 commented Jan 2, 2023 •

edited

Loading

Cube707 left a comment •

edited

Loading

Cube707 Jan 8, 2023 •

edited

Loading

Cube707 Jan 8, 2023 •

edited

Loading

NivEz commented Feb 3, 2023 •

edited

Loading