Replace text inputs with checkboxes in the bootstrap wizard UI #2931
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
opudrovs
force-pushed
the
2870-replace-text-inputs-with-checkboxes-in-bootstrap-wizard-ui
branch
4 times, most recently
from
79b4471 to
e21e63d
Compare
opudrovs
changed the title
bootstrapWizardInput and related types.
opudrovs
force-pushed
the
2870-replace-text-inputs-with-checkboxes-in-bootstrap-wizard-ui
branch
from
e21e63d to
29eee0e
Compare
Replace text fields with checkboxes for Boolean values in the bootstrap wizard. Add local variables for input lookup. Removed unnecessary copying of styles.
opudrovs
force-pushed
the
2870-replace-text-inputs-with-checkboxes-in-bootstrap-wizard-ui
branch
from
29eee0e to
713aeb1
Compare
opudrovs
added
type/enhancement
|
Tested that params are built properly with checkboxes. |
ozamosi
approved these changes
Oct 27, 2022
Comment on lines
453
to
454
| for i := range m.inputs { | ||
| input := m.inputs[i] |
There was a problem hiding this comment.
💡
Suggested change
| for i := range m.inputs { | |
| input := m.inputs[i] | |
| for i, input := range m.inputs { |
There's a few other for loops that can be simplified by just grabbing the second argument from range.
There was a problem hiding this comment.
Good catch! JS habits die hard. 🤣
There was a problem hiding this comment.
Refactored the loops.
| } | ||
|
|
||
| return ti | ||
| return fmt.Sprintf("%s%s%s", open, checkmark, close) |
There was a problem hiding this comment.
[x] I panic that I don't know what it's asking me to do and refuse to read the help text above until I've figured it out, whereas [x] private would calm me down enough to pay attention.
There was a problem hiding this comment.
Added checkbox placeholders.
opudrovs
deleted the
2870-replace-text-inputs-with-checkboxes-in-bootstrap-wizard-ui
branch
This was referenced Oct 27, 2022
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #2870
Added type
bootstrapWizardInputand related types.Replaced text fields with checkboxes for Boolean values in the bootstrap wizard. Checkboxes can be toggles with the Space key when focused.
Minor refactoring:
-- added local variables for input lookup (
input := inputs[i]) for readability-- removed unnecessary copying of styles. I previously copied it from Bubble Tea examples, so I'm not sure why
style.Copy()was used not only when creating new styles based on old styles but also when just rendering elements with in this style. So, removed it and tested it locally, it seems to work fine without the redundant copying. If anything goes wrong, we can always put it back, but it should work fine.Questions:
Checkbox toggled with space:
Greyed out underscore marks false value. The color of the underscore can be changed or or the underscore can be removed easily if needed.
Focuses checkbox with true value + unfocused checkbox: