Skip to content

#641 Moved options rendering on check you data to before the data tabs. #644

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.

Sign up for GitHub

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

Open
davidgouge wants to merge 2 commits into
base: main
Choose a base branch
from
+96 −80
Open

#641 Moved options rendering on check you data to before the data tabs. #644

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3990f21
#641 Moved options rendering on check you data to before the data tabs.
davidgouge Oct 19, 2026
dfdc806
Merge branch 'main' into 641-checkdata-page-layout-change
davidgouge Oct 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
159 changes: 80 additions & 79 deletions src/DfE.CheckPerformanceData.Web/Views/CheckYourPupilData/Index.cshtml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,86 @@
CSV files as a ZIP file</a>
</p>

@* #317: the form offers whatever the open exercises offer. Nothing open means no form at all —
the tables, the search and the downloads are untouched, because a closed exercise removes
actions, never content. #641: the options sit above the tables, so a school sees what it can do
without scrolling past them. AB#298317: when results enquiry is the only thing left, the page asks
the designed Yes/No question instead of showing a one-item form. *@
@if (Model.OffersEnquiryOnly)
{
<form method="post" action="@Url.Action("NextStep", "CheckYourPupilData", new { windowId = Model.WindowId })">
@Html.AntiForgeryToken()
<govuk-radios for="SelectedNextStep">
<govuk-radios-fieldset>
@* Not is-page-heading: the EditableTitle above is the page's only h1. *@
<govuk-radios-fieldset-legend class="govuk-fieldset__legend--l">
Would you like to report an issue with an exam result?
</govuk-radios-fieldset-legend>

<govuk-radios-hint>
Select one option
</govuk-radios-hint>

<govuk-radios-item value="@NextSteps.ResultsEnquiry">Yes</govuk-radios-item>
<govuk-radios-item value="@NextSteps.SignOut">No, I'd like to sign out of this service</govuk-radios-item>
</govuk-radios-fieldset>
</govuk-radios>

<govuk-button type="submit">Continue</govuk-button>
</form>
}
else if (Model.AvailableNextSteps.Count == 1)
{
@* A radio group with a single choice is a poor pattern and contradicts the "select one option"
hint, so the one surviving option becomes the button itself. The value still posts through a
hidden field, so NextStep re-derives and re-checks it exactly as it does for the radios.
Unreachable today (pupil data always offers two options and results enquiry is handled
above); kept for a future exercise type that offers exactly one. *@
var only = Model.AvailableNextSteps[0];

<form method="post" action="@Url.Action("NextStep", "CheckYourPupilData", new { windowId = Model.WindowId })">
@Html.AntiForgeryToken()
<input type="hidden" name="@nameof(Model.SelectedNextStep)" value="@only"/>
<govuk-button type="submit">@NextStepLabels.For(only, Model.LearnerNoun)</govuk-button>
</form>
}
else if (Model.AvailableNextSteps.Count > 1)
{
<form method="post" action="@Url.Action("NextStep", "CheckYourPupilData", new { windowId = Model.WindowId })">
@Html.AntiForgeryToken()
<govuk-radios for="SelectedNextStep">
<govuk-radios-fieldset>
@* Not is-page-heading: that renders the legend as an <h1>, and this page already
has one (the check-pupil-data-title EditableTitle above). Two <h1>s leave the
document with no single unambiguous title. The legend keeps the --l size so it
looks unchanged. *@
<govuk-radios-fieldset-legend class="govuk-fieldset__legend--l">
What would you like to do?
</govuk-radios-fieldset-legend>

<govuk-radios-hint>
Select one option.
</govuk-radios-hint>

@* "Confirm ... data is correct" is the alternative to doing anything, so it reads
last, after the "or" divider — the GDS pattern, and the order this page has always
rendered. Every other option keeps the service's exercise-sort order. *@
@foreach (var step in Model.AvailableNextSteps.Where(s => s != NextSteps.Confirm))
{
<govuk-radios-item value="@step">@NextStepLabels.For(step, Model.LearnerNoun)</govuk-radios-item>
}
@if (Model.AvailableNextSteps.Contains(NextSteps.Confirm))
{
<govuk-radios-divider>or</govuk-radios-divider>
<govuk-radios-item value="@NextSteps.Confirm">@NextStepLabels.For(NextSteps.Confirm, Model.LearnerNoun)</govuk-radios-item>
}
</govuk-radios-fieldset>
</govuk-radios>

<govuk-button type="submit">Continue</govuk-button>
</form>
}

@{
// Every section on the page, so a query string rebuilt from any one tab (search, paging)
// carries the others' page and search — the Results tab included, when there is one.
Expand Down Expand Up @@ -160,82 +240,3 @@ else
}
</govuk-tabs>
}

@* #317: the form offers whatever the open exercises offer. Nothing open means no form at all —
the tables, the search and the downloads above are untouched, because a closed exercise removes
actions, never content. AB#298317: when results enquiry is the only thing left, the page asks
the designed Yes/No question instead of showing a one-item form. *@
@if (Model.OffersEnquiryOnly)
{
<form method="post" action="@Url.Action("NextStep", "CheckYourPupilData", new { windowId = Model.WindowId })">
@Html.AntiForgeryToken()
<govuk-radios for="SelectedNextStep">
<govuk-radios-fieldset>
@* Not is-page-heading: the EditableTitle above is the page's only h1. *@
<govuk-radios-fieldset-legend class="govuk-fieldset__legend--l">
Would you like to report an issue with an exam result?
</govuk-radios-fieldset-legend>

<govuk-radios-hint>
Select one option
</govuk-radios-hint>

<govuk-radios-item value="@NextSteps.ResultsEnquiry">Yes</govuk-radios-item>
<govuk-radios-item value="@NextSteps.SignOut">No, I'd like to sign out of this service</govuk-radios-item>
</govuk-radios-fieldset>
</govuk-radios>

<govuk-button type="submit">Continue</govuk-button>
</form>
}
else if (Model.AvailableNextSteps.Count == 1)
{
@* A radio group with a single choice is a poor pattern and contradicts the "select one option"
hint, so the one surviving option becomes the button itself. The value still posts through a
hidden field, so NextStep re-derives and re-checks it exactly as it does for the radios.
Unreachable today (pupil data always offers two options and results enquiry is handled
above); kept for a future exercise type that offers exactly one. *@
var only = Model.AvailableNextSteps[0];

<form method="post" action="@Url.Action("NextStep", "CheckYourPupilData", new { windowId = Model.WindowId })">
@Html.AntiForgeryToken()
<input type="hidden" name="@nameof(Model.SelectedNextStep)" value="@only"/>
<govuk-button type="submit">@NextStepLabels.For(only, Model.LearnerNoun)</govuk-button>
</form>
}
else if (Model.AvailableNextSteps.Count > 1)
{
<form method="post" action="@Url.Action("NextStep", "CheckYourPupilData", new { windowId = Model.WindowId })">
@Html.AntiForgeryToken()
<govuk-radios for="SelectedNextStep">
<govuk-radios-fieldset>
@* Not is-page-heading: that renders the legend as an <h1>, and this page already
has one (the check-pupil-data-title EditableTitle above). Two <h1>s leave the
document with no single unambiguous title. The legend keeps the --l size so it
looks unchanged. *@
<govuk-radios-fieldset-legend class="govuk-fieldset__legend--l">
What would you like to do?
</govuk-radios-fieldset-legend>

<govuk-radios-hint>
Select one option.
</govuk-radios-hint>

@* "Confirm ... data is correct" is the alternative to doing anything, so it reads
last, after the "or" divider — the GDS pattern, and the order this page has always
rendered. Every other option keeps the service's exercise-sort order. *@
@foreach (var step in Model.AvailableNextSteps.Where(s => s != NextSteps.Confirm))
{
<govuk-radios-item value="@step">@NextStepLabels.For(step, Model.LearnerNoun)</govuk-radios-item>
}
@if (Model.AvailableNextSteps.Contains(NextSteps.Confirm))
{
<govuk-radios-divider>or</govuk-radios-divider>
<govuk-radios-item value="@NextSteps.Confirm">@NextStepLabels.For(NextSteps.Confirm, Model.LearnerNoun)</govuk-radios-item>
}
</govuk-radios-fieldset>
</govuk-radios>

<govuk-button type="submit">Continue</govuk-button>
</form>
}
17 changes: 16 additions & 1 deletion tests/DfE.CheckPerformanceData.UnitTests/Web/CheckYourPupilDataViewRenderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public void The_form_is_driven_by_the_available_options_not_by_the_window_being_
public void No_available_option_renders_no_form_at_all()
{
// AB#298317: the closed statement moved up into the intro paragraph (see
// The_closed_window_paragraph_…), so nothing renders below the tables when nothing is open.
// The_closed_window_paragraph_…), so no form renders when nothing is open.
var view = ReadView();

Assert.DoesNotContain("closed for changes", view);
Expand All @@ -45,6 +45,21 @@ public void No_available_option_renders_no_form_at_all()
Assert.True(firstBranch >= 0 && firstForm > firstBranch, "no form may render before the option branches");
}

[Fact]
public void The_options_render_after_the_download_all_link_and_before_the_tabs()
{
// #641: title, intro, Download all, then the call to action, then the tables.
var view = ReadView();

var downloadAll = view.IndexOf("asp-action=\"DownloadAll\"", StringComparison.Ordinal);
var firstBranch = view.IndexOf("@if (Model.OffersEnquiryOnly)", StringComparison.Ordinal);
var lastForm = view.LastIndexOf("</form>", StringComparison.Ordinal);
var firstTabs = view.IndexOf("<govuk-tabs>", StringComparison.Ordinal);

Assert.True(downloadAll >= 0 && firstBranch > downloadAll, "the options render after the Download all link");
Assert.True(firstTabs >= 0 && lastForm < firstTabs, "every option form renders before the tabs");
}

[Fact]
public void A_single_available_option_renders_a_button_not_a_one_item_radio_group()
{
Expand Down
Loading