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

Unlisted choice improvements #776

Closed

Conversation

batpad
Copy link
Contributor

@batpad batpad commented Sep 2, 2023

Towards #756 and #757 -

  • Allows for an other_text option inside unlisted_choice which allows user to specify a custom text in the select drop-down
  • Hide select drop-down and label if no choices are defined
  • Handles no choices in backend processing

I might have underestimated the complexity in hiding the select dropdown if there are no choices present, and that bit seems to have gotten a bit ugly. Will leave some comments in the code to see if there's things we can improve.

@yuvipanda @consideRatio @GeorgianaElena - firstly, thank you so much for fixing up issues with the unlisted_choices work, and many apologies for the delay in getting to these fixes. I am not super happy with the implementation in this PR, and I'll leave some more notes with the code. I did a bit of testing though, and it seems to me like it all works as expected.

If this approach seems ok to you all, I think what might be pending:

  • Create an example profile option to not have any choices but have an unlisted_choice to make testing easier
  • Create a test with no choices to make sure the backend handling is covered with tests.

 - Allows for an `other_text` option inside `unlisted_choice` which allows user to specify a custom text in the select drop-down
 - Hide select drop-down and label if no choices are defined
@batpad
Copy link
Contributor Author

batpad commented Sep 2, 2023

Oops, my bad - I thought I was up-to-date with kubespawner:main but apparently not. Just going to close with PR and open a new one cleaned up. Apologies for the noise.

@batpad batpad closed this Sep 2, 2023
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.

1 participant