-
Notifications
You must be signed in to change notification settings - Fork 1
Redo SingleChoiceSelection for better differentiation between required and optional selections #222
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
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
a71db09
add Optional SingleChoice fields and do not allow empty selection in …
patritzenfeld 3207315
fix nesting and doctest examples
patritzenfeld 4d37317
get style of empty option in line with others
patritzenfeld b3c0f9f
remove empty answer from SingleChoiceSelection and deprecate associat…
patritzenfeld d6b496d
allow html tag spelling
patritzenfeld 92f5583
adjust comment
patritzenfeld 0a5bc13
simplify Formify instances for SingleChoice
patritzenfeld 0d36908
fix empty selection value for radio buttons
patritzenfeld 29e22d2
add instance helper for optional selection forms
patritzenfeld 64d4732
write out required attribute in example
patritzenfeld b365bb7
relax max line length
patritzenfeld 6eb7451
adjust error message
patritzenfeld File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ hasdata | |
| img | ||
| lindex | ||
| lucius | ||
| optgroup | ||
| RWS | ||
| syb | ||
| subdirs | ||
|
|
||
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
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
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
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
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should probably be
value=""so I can make use of
<select required>to get the browser's own pre-submit checks like this:https://stackoverflow.com/a/6048891
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should already be the case if you use
SingleChoiceSelection. Thisvalue="None"is only offered for form typeMaybe SingleChoiceSelection. If the field is required, then this is used instead:flex-tasks/flex-tasks/src/FlexTask/Widgets.hs
Lines 141 to 145 in 14342ca
But I guess I should revise my custom
selectFieldfunction to be less confusing. I just did a quick copy and edit on the standardYesodversion which resulted in messy code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently, with
formify (Nothing @SingleChoiceSelection), I'll get a dropdown where the value defaults to the first real value, and the None option is disabled:I want it to default to
Noneinstead (the table is big so making it default to None makes it easy for students to see which dropdown they have not yet answered), but I also want the required check to take effect.For this to work, perhaps what I need is just removing
disabledfrom https://github.com/fmidue/flex-tasks/pull/222/changes#diff-26588820d6d6d6eb9930e0b89192489214299acf12c3b5043f1b362e02333b7bR774? what do you think ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I guess the behaviour is different between browsers. It did initially select the None option when I tried it on my setup, but did not allow reselecting it. I I will just remove the disabled attribute then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What browser are you using? I'd like to take a look at this again on Tuesday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
firefox