HEEDLS-377 - edit centre manger details implemented and some detail l… #329
Conversation
…ist styling commonised
DigitalLearningSolutions.Web/ViewModels/TrackingSystem/CentreManagerDetailsViewModel.cs
Outdated
Show resolved
Hide resolved
...alLearningSolutions.Web/Views/TrackingSystem/CentreConfiguration/CentreManagerDetails.cshtml
Outdated
Show resolved
Hide resolved
| <button class="nhsuk-button nhsuk-u-margin-top-4" type="submit" value="save">Save</button> | ||
| </form> | ||
|
|
||
| <div class="nhsuk-back-link"> |
There was a problem hiding this comment.
There is a back link view component that you can use instead of this (unless for some reason this one is different)
There was a problem hiding this comment.
This one is mildly different as it uses "Cancel" as the link text rather than "Go Back". I was contemplating modifying the back link component to take in some link text in the model. Originally, I decided not to as:
- Putting the link text as an optional parameter wouldn't work as tag helpers don't seem to understand optional parameters ( ViewComponentTagHelper: Allow optional parameters dotnet/aspnetcore#5011 ). So whenever the Go Back component was used, we'd have to specify the text.
- I think the vast majority of cases will probably just be "Go Back", so it will be more simple to not have to specify the text.
Though now I am working on other centre detail editing tickets, I see they will also want a Cancel back link, so I don't know if making a new Cancel Link view component (that would basically be identical to the back link, except for the text) would be preferred, or modifying the back link to take in link text would be easier.
There was a problem hiding this comment.
OK, the NHS back link is the styling around the text, so if you do find you want to make a component, I think changing the existing one to accept a parameter would probably be better. They require the parameters to be specified, but there is nothing stopping us defining it such that if you enter no text for the parameter (i.e. text="") then it would default to Go Back
There was a problem hiding this comment.
I think we should either create a new VC with "Cancel" text as it will be used in loads of places, or we can modify the back button to take text as a parameter (either Cancel or Go back - I think having a default might be confusing if text is set to "")
DigitalLearningSolutions.Data.Tests/DataServices/CentresDataServiceTests.cs
Outdated
Show resolved
Hide resolved
|
Just checked the line endings in CentreConfigurationController - they are all consistently CRLF. |
DigitalLearningSolutions.Data/DataServices/CentresDataService.cs
Outdated
Show resolved
Hide resolved
DigitalLearningSolutions.Web/Controllers/TrackingSystem/CentreConfigurationController.cs
Outdated
Show resolved
Hide resolved
DigitalLearningSolutions.Data.Tests/DataServices/CentresDataServiceTests.cs
Show resolved
Hide resolved
DigitalLearningSolutions.Web/ViewComponents/BackLinkViewComponent.cs
Outdated
Show resolved
Hide resolved
…ntre data service method name
| <vc:error-summary order-of-property-names="@(new[] { nameof(Model.FirstName), nameof(Model.LastName), nameof(Model.Email), nameof(Model.Telephone) })"/> | ||
| } | ||
|
|
||
| <h1 id="page-heading" class="nhsuk-heading-xl">Centre manager details</h1> |
There was a problem hiding this comment.
Super minor, but could we change the h1 + title to "Edit XXX"?
DanBloxham-sw
deleted the
HEEDLS-377-CentreConfigurationEditCentreManagerDetails
branch
…ist styling commonised
Screen shots:
Regular page:
With errors:
Mobile:
