diff options
author | Matthew Somerville <matthew@mysociety.org> | 2019-08-30 15:53:31 +0100 |
---|---|---|
committer | Matthew Somerville <matthew@mysociety.org> | 2019-09-05 12:00:09 +0100 |
commit | b8916dcf8e8eeefea426a4725601bbd22263848e (patch) | |
tree | 397e6d5e7626a49f8f051ec7286ff02183dfaa9d | |
parent | cd405cc208bd1efa36c0ab6e7d5af7e4e56be733 (diff) |
Improve user flow when JavaScript is not available
This improves the reporting journey to only ask for category,
and then category extra questions if appropriate, first, so that
if the choice would lead to the form being disabled, this can
be shown immediately.
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 52 | ||||
-rw-r--r-- | t/app/controller/report_new_open311.t | 47 | ||||
-rw-r--r-- | templates/web/base/report/new/category_extras_fields.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/new/category_wrapper.html | 6 | ||||
-rw-r--r-- | templates/web/base/report/new/form_report.html | 16 | ||||
-rw-r--r-- | templates/web/base/report/new/form_title.html | 2 | ||||
-rw-r--r-- | templates/web/base/report/new/form_user.html | 2 |
7 files changed, 119 insertions, 8 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index dd7f3f5fa..bc19d9c8b 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -1131,7 +1131,7 @@ sub set_report_extras : Private { foreach my $item (@metalist) { my ($metas, $param_prefix) = @$item; foreach my $field ( @$metas ) { - if ( lc( $field->{required} ) eq 'true' && !$c->cobrand->category_extra_hidden($field)) { + if ( lc( $field->{required} || '' ) eq 'true' && !$c->cobrand->category_extra_hidden($field)) { unless ( $c->get_param($param_prefix . $field->{code}) ) { $c->stash->{field_errors}->{ 'x' . $field->{code} } = _('This information is required'); } @@ -1518,10 +1518,56 @@ sub generate_map : Private { sub check_for_category : Private { my ( $self, $c ) = @_; - my $category = $c->get_param('category') || ''; + my $category = $c->get_param('category') || $c->stash->{report}->category || ''; $category = '' if $category eq _('Loading...') || $category eq _('-- Pick a category --'); - $c->stash->{category} = $category || $c->stash->{report}->category || ''; + $c->stash->{category} = $category; + # Bit of a copy of set_report_extras, because we need the results here, but + # don't want to run all of that fn until later as it e.g. alters field + # errors at that point. Also, the report might already have some answers in + # too if e.g. gone via social login... TODO Improve this? + my $extra = $c->stash->{report}->get_extra_fields; + my %current = map { $_->{name} => $_ } @$extra; + + my @contacts = grep { $_->category eq $category } @{$c->stash->{contacts}}; + my @metalist = map { @{$_->get_metadata_for_storage} } @contacts; + my @extra; + foreach my $field (@metalist) { + push @extra, { + name => $field->{code}, + description => $field->{description}, + value => $c->get_param($field->{code}) || $current{$field->{code}}{value} || '', + }; + } + $c->stash->{report}->set_extra_fields( @extra ); + + # Work out if the selected category (or category extra question answer) should lead + # to a message being shown not to use the form + if ( $c->stash->{category_extras}->{$category} && @{ $c->stash->{category_extras}->{$category} } >= 1 ) { + my $disable_form_messages = $c->forward('disable_form_message'); + if ($disable_form_messages->{all}) { + $c->stash->{disable_form_message} = $disable_form_messages->{all}; + } elsif (my $code = $disable_form_messages->{code}) { + my $answer = $c->get_param($code); + my $message = $disable_form_messages->{message}; + if ($answer) { + foreach (@{$disable_form_messages->{answers}}) { + if ($answer eq $_) { + $c->stash->{disable_form_message} = $message; + } + } + } else { + $c->stash->{have_disable_qn_to_answer} = 1; + } + } + } + + if ($c->get_param('submit_category_part_only') || $c->stash->{disable_form_message}) { + # If we've clicked the first-part category button (no-JS only probably), + # or the category submitted will be showing a disabled form message, + # we only want to reshow the form + $c->stash->{force_form_not_submitted} = 1; + } } =head2 redirect_or_confirm_creation diff --git a/t/app/controller/report_new_open311.t b/t/app/controller/report_new_open311.t index b52e0af18..52f1ddc6e 100644 --- a/t/app/controller/report_new_open311.t +++ b/t/app/controller/report_new_open311.t @@ -62,6 +62,12 @@ my $contact4 = $mech->create_contact_ok( { description => 'Asset ID', code => 'central_asset_id', required => 'true', automated => 'hidden_field', variable => 'true', order => '2' }, ] }, ); +# Another one to switch to in disable form test +$mech->create_contact_ok( + body_id => $body2->id, # Edinburgh + category => 'Something Other', + email => '104', +); # test that the various bit of form get filled in and errors correctly # generated. @@ -310,6 +316,47 @@ subtest "Category extras includes form disabling string" => sub { answers => [ 'yes' ], }; } + + # Test new non-JS form disabling flow + $mech->get_ok('/report/new?latitude=55.952055&longitude=-3.189579'); + $mech->content_contains('name="submit_category_part_only"'); + $mech->submit_form_ok({ with_fields => { category => 'Pothole' } }); + $mech->content_contains('<div id="js-category-stopper" class="box-warning" role="alert" aria-live="assertive">'); + $mech->content_contains('Please ring us!'); + # Switch to another, okay, category + $mech->submit_form_ok({ with_fields => { category => 'Something Other' } }); + $mech->content_lacks('<div id="js-category-stopper" class="box-warning" role="alert" aria-live="assertive">'); + $mech->content_lacks('Please ring us!'); + + # Remove the required extra field so its error checking doesn't get in the way + my $extra = $contact4->get_extra_fields; + @$extra = grep { $_->{code} ne 'size' } @$extra; + $contact4->set_extra_fields(@$extra); + $contact4->update; + + # Test submission of whole form, switching back to a blocked category at the same time + $mech->submit_form_ok({ with_fields => { + category => 'Pothole', title => 'Title', detail => 'Detail', + username => 'testing@example.org', name => 'Testing Example', + } }); + $mech->content_contains('<div id="js-category-stopper" class="box-warning" role="alert" aria-live="assertive">'); + $mech->content_contains('Please ring us!'); + + # Test special answer disabling of form + $extra = $contact4->get_extra_fields; + @$extra = grep { $_->{code} ne 'ring' } @$extra; # Remove that all-category one + $contact4->set_extra_fields(@$extra); + $contact4->update; + $mech->get_ok('/report/new?latitude=55.952055&longitude=-3.189579'); + $mech->content_contains('name="submit_category_part_only"'); + $mech->submit_form_ok({ with_fields => { category => 'Pothole' } }); + $mech->content_contains('name="submit_category_part_only"'); + $mech->submit_form_ok({ with_fields => { dangerous => 'no' } }); + $mech->content_lacks('<div id="js-category-stopper" class="box-warning" role="alert" aria-live="assertive">'); + $mech->content_lacks('Please please ring'); + $mech->submit_form_ok({ with_fields => { dangerous => 'yes' } }); + $mech->content_contains('<div id="js-category-stopper" class="box-warning" role="alert" aria-live="assertive">'); + $mech->content_contains('Please please ring'); }; }; diff --git a/templates/web/base/report/new/category_extras_fields.html b/templates/web/base/report/new/category_extras_fields.html index dd5c3911d..e9237f83b 100644 --- a/templates/web/base/report/new/category_extras_fields.html +++ b/templates/web/base/report/new/category_extras_fields.html @@ -1,6 +1,6 @@ [%- FOR meta IN metas %] [%- meta_name = meta.code -%] - [%- x_meta_name = 'x' _ meta.code # For report_meta and field_erros lookup, as TT hides codes starting "_" -%] + [%- x_meta_name = 'x' _ meta.code # For report_meta and field_errors lookup, as TT hides codes starting "_" -%] [% IF c.cobrand.category_extra_hidden(meta) AND NOT show_hidden %] diff --git a/templates/web/base/report/new/category_wrapper.html b/templates/web/base/report/new/category_wrapper.html index 6cbb55229..32785b450 100644 --- a/templates/web/base/report/new/category_wrapper.html +++ b/templates/web/base/report/new/category_wrapper.html @@ -19,6 +19,11 @@ [% PROCESS "report/new/duplicate_suggestions.html" %] +[% IF disable_form_message %] +<div id="js-category-stopper" class="box-warning" role="alert" aria-live="assertive"> + [% disable_form_message %] +</div> +[% ELSE %] <div id="js-post-category-messages" class="js-hide-if-invalid-category_extras"> [%# This section includes 'Pick an asset' text, roadworks info, extra category questions %] @@ -26,3 +31,4 @@ [% PROCESS "report/new/category_extras.html" %] [%- END %] </div> +[% END %] diff --git a/templates/web/base/report/new/form_report.html b/templates/web/base/report/new/form_report.html index e5facc305..52133ba68 100644 --- a/templates/web/base/report/new/form_report.html +++ b/templates/web/base/report/new/form_report.html @@ -1,3 +1,5 @@ +[% SET form_show_category_only = NOT category || field_errors.category || disable_form_message || have_disable_qn_to_answer %] + <!-- report/new/form_report.html --> [% INCLUDE 'report/new/form_heading.html' %] @@ -7,7 +9,17 @@ [% PROCESS "report/new/category_wrapper.html" %] [% TRY %][% PROCESS 'report/new/after_category.html' %][% CATCH file %][% END %] -<div class="js-hide-if-invalid-category"> + +[% IF form_show_category_only %] + <div class="hidden-js"> + <input class="btn btn--primary btn--block btn--final" type="submit" name="submit_category_part_only" value="[% loc('Submit') %]"> + </div> + [% SET field_required = '' %] +[% ELSE %] + [% SET field_required = ' required' %] +[% END %] + +<div class="js-hide-if-invalid-category[% ' hidden-nojs' IF form_show_category_only %]"> [% TRY %][% PROCESS 'report/new/_form_labels.html' %][% CATCH file %][% END %] <h2 class="form-section-heading js-hide-if-private-category">[% loc( 'Public details' ) %]</h2> @@ -67,7 +79,7 @@ <p class='form-error'>[% field_errors.detail %]</p> [% END %] - <textarea class="form-control" rows="7" cols="26" name="detail" id="form_detail" [% IF form_detail_placeholder %]aria-describedby="detail-hint"[% END %] required>[% report.detail | html %]</textarea> + <textarea class="form-control" rows="7" cols="26" name="detail" id="form_detail" [% IF form_detail_placeholder %]aria-describedby="detail-hint"[% END %][% field_required %]>[% report.detail | html %]</textarea> [% TRY %][% PROCESS 'report/new/inline-tips.html' %][% CATCH file %][% END %] diff --git a/templates/web/base/report/new/form_title.html b/templates/web/base/report/new/form_title.html index a20d836ac..423a0cf0e 100644 --- a/templates/web/base/report/new/form_title.html +++ b/templates/web/base/report/new/form_title.html @@ -5,4 +5,4 @@ [% IF field_errors.title %] <p class='form-error'>[% field_errors.title %]</p> [% END %] -<input class="form-control" type="text" value="[% report.title | html %]" name="title" id="form_title" aria-describedby="title-hint" required autocomplete="off"> +<input class="form-control" type="text" value="[% report.title | html %]" name="title" id="form_title" aria-describedby="title-hint"[% field_required %] autocomplete="off"> diff --git a/templates/web/base/report/new/form_user.html b/templates/web/base/report/new/form_user.html index 49ef70abd..73e1b5d33 100644 --- a/templates/web/base/report/new/form_user.html +++ b/templates/web/base/report/new/form_user.html @@ -1,5 +1,5 @@ <!-- report/new/form_user.html --> -<div class="js-hide-if-invalid-category"> +<div class="js-hide-if-invalid-category[% ' hidden-nojs' IF form_show_category_only %]"> [% PROCESS 'report/form/user.html' type='report' %] |