Components refactor#233
Draft
stusherwin wants to merge 7 commits into
Draft
Conversation
16759b0 to
1642993
Compare
Deployments
|
1642993 to
940fb74
Compare
…comparison headline measures
940fb74 to
657f410
Compare
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
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.
This PR is the result of the Components spike, resulting in a number of proposed changes:
1. <tabbed-view> tag helper
This encapsulates the switchable tabs within an individual measure as displayed on a page:
This produces the HTML (simplified):
Which results in the following:

2. Measure components (partials)
A number of measure-related partials have been introduced which split up the view code needed to render the component parts of a measure displayed on the page.
_MeasurePartial:
_MeasureView model:
MeasureViewModelRenders the complete measure on the page, with its submeasures:
Note the submeasures (three year average, top performers etc.) are rendered dynamically as different
<tab-content>elements. Different types of measures can have different submeasures, for instance a KS4 headline measure for a school will show Three year average, Top performers, Year by Year and Table submeasures, whereas the same KS4 headline measure when comparing a school with one of its similar schools will only show Three year average, Year by Year and Table submeasures._MeasureFiltersPartial:
_MeasureFiltersView model:
MeasureInfoViewModelDynamically renders any filter dropdowns attached to the measure:
_MeasureThreeYearAverageChartPartial:
_MeasureThreeYearAverageChartView model:
ThreeYearAverageSubMeasureViewModelRenders the three year average submeasure:

Note the number of averages and the labels associated are now dynamic, for instance when rendered within a measure on a school page, the school average, similar schools average, LA average and England average are shown. Whereas on a school comparison page, the current school average, similar school average and England average are shown.
_MeasureTopPerformersPartial:
_MeasureTopPerformersView model:
TopPerformersSubMeasureViewModelRenders the top perfomers submeasure:

_MeasureYearByYearChartPartial:
_MeasureYearByYearChartView model:
YearByYearSubMeasureViewModelRenders the year-by-year submeasure:

Note the year-by-year series and the labels associated are now dynamic, for instance when rendered within a measure on a school page, the school average, similar schools average, LA average and England average are shown. Whereas on a school comparison page, the current school average, similar school average and England average are shown.
_MeasureTablePartial:
_MeasureTableView model:
TableSubMeasureViewModelRenders the table submeasure:

Note the rows and row headers are now dynamic, for instance when rendered within a measure on a school page, the school average, similar schools average, LA average and England average are shown. Whereas on a school comparison page, the current school average, similar school average and England average are shown.
3. Measure filtering
To make measures more generic, the functionality for filtering has been simplified. Previously there was a separate controller action for each measure type (e.g. English and Maths, Destinations, Attendance, GCSE Core Subject), which applied the filter selected in the dropdown and returned a JSON object containing all the values for each submeasure. This was then applied to the page using some custom JavaScript code which altered the values of the HTML elements for each submeasure.
This has been changed so that there is now a single action for each measure page. This action also handles filtering for each measure within the page - filters can now be applied at the page level with a querystring, so for example the URL /school/136994/ks4-core-subjects?english-literature:grade=5&biology:grade=7 will apply filters simultaneously to the English literature and Biology measures:
The JavaScript code to apply a filter when selected now just requests the whole page again via the Fetch API with the new filter applied to the querystring, and replaces the content of the measure with the same id from the response content.
This also means that when we come to ensure filtering functionality still works without JavaScript enabled, it should be a simple change to allow a form to submit the whole page with the selected filters applied.
Note there is a small issue that still needs resolving, where if the user selects a tab within a measure, and then applies the filter for that measure, it will forget what tab they were on. This should be fairly straightforward to fix, I just ran out of time in this spike.
4. Use case refactoring
In order to implement the UI changes, a couple of refactorings needed to be done at the use case level:
1. Pull filtering logic out of the controllers and into the use cases
This has a dual purpose:
The relevant use cases (
GetSchoolKs4HeadlineMeasures,GetSchoolKs4CoreSubjects,GetSchoolComparisonKs4HeadlineMeasures,GetSchoolComparisonKs4CoreSubjects) now take an additionalIDictionary<string, string>? FilterByparameter in their requests, and pass the filters on to each individualMeasureobject to do its own filtering.2. Extract out the concept of a
Measureinto its own domain object, and have each use case return a list of theseMeasureobjects.The previous version of the code did not have a concept of a measure modelled explicitly, and each use case had a different way of querying the measure data and structuring it in its responses. Each measure now is represented by the
Measuredomain object, which represents a collection ofSubMeasures.Each
MeasureandSubMeasureclass has the responsibility of building out its own data depending on whether it is a "school" measure or a "school comparison" measure (theForSchool()andForSchoolComparison()static methods).Measures also now have builder methods which are grouped into the
Ks4HeadlineMeasuresandKs4CoreSubjectsstatic classes:These can then be called from the use cases as follows:
5. JavaScript file restructure
The new implementation had a bug where the current tab in a measure would not be remembered when a new filter value was applied. The fix would involve some changes to the
similar-schools-tabs-mobile-fix.jsfile, and while investigating this it seemed that the purpose of this file was to override the mobile behaviour of the tabs to only show the currently selected tab rather than list out all the tabs in the page. This was implemented by effectively re-implementing the behaviour of thegovuk-tabscomponent. However it seemed that this could be replaced by a simple override to the behaviour of the component - implemented in themobile-collapsed-tabsmodule](https://github.com/DFE-Digital/sap-sector/pull/233/changes#diff-13f6c8a3203c56d77df17153eeebd28e6d38db0f5879d60345a8889f1decde5c).In order to accomplish this, the structure of the existing JS files needed to be changed to module files rather than standalone scripts. This then meant that modules could be included within other modules, for example the
measure-filtersmodule now includes themobile-collapsed-tabsmodule and callsMobileCollapsedTabs.selectTabById()to re-select the current tab after the filters have been applied.Summary
The changes above have been implemented in the following pages (using example URNs):
School headline measures/core subjects pages
/school/136994/ks4-headline-measures
/school/136994/ks4-core-subjects
School comparison headline measures/core subjects pages
/school/136994/view-similar-schools/137921/ks4-core-subjects
/school/136994/view-similar-schools/137921/ks4-headline-measures
I think the best approach if we agree this is the way to go would be to implement these changes bit by bit (say a separate PR for each page affected) - that way we can review as we go and ensure each change is fully tested.