Snagging updates #852
Snagging updates #852
Conversation
gpeng
left a comment
gpeng
left a comment
There was a problem hiding this comment.
Good stuff. I've added a couple of comments. I know you've only opened it as draft but I'm off now. @malcolmbaig can give you a hand next week. Could you also rebase from main instead of merging to avoid that 🤮 merge commit 😁
this has been changed to fit in with design standards as the date/time font is a secondary text
As all of our clinical types will be Screening, this column is redundant information
Added to fit in line with the prototype after snagging session
cameronhargreaves1-nhs
force-pushed
the
DTOSS-11864-snagging-updates
branch
from
cde8cea to
329adb6
Compare
This has been changed as if we collect all clinics, this list will grow very large and will get out of control very quick. This has been changed to the last 7 days and can be more digestable
cameronhargreaves1-nhs
force-pushed
the
DTOSS-11864-snagging-updates
branch
from
329adb6 to
3c21535
Compare
|
| self.page.goto(self.live_server_url + reverse("clinics:index")) | ||
| self.assert_page_title_contains("Today’s clinics") | ||
| heading = self.page.get_by_role("heading", level=1) | ||
| expect(heading).to_contain_text("provider") |
There was a problem hiding this comment.
Our system specs are set up so that if you've called a step which invokes the login_as_user function (so in this file it would be the step self.given_i_am_logged_in_as_a_clinical_user()), you'll have access to the following in your test:
self.current_user
self.current_provider
A small improvement on the assertion above would be:
| expect(heading).to_contain_text("provider") | |
| expect(heading).to_contain_text(self.current_provider.name) |
|
Commit messages look good. A useful convention for the message body is to format it with a max width of 72 characters. This keeps the git log easy to read (especially when viewed in narrow windows/splits) and consistent with the rest of the commit history. You can set up whichever editor you use for commit messages to enforce the 72 char width. |
| {{ presented_clinic.type }} | ||
| <br> | ||
| <span class="nhsuk-u-secondary-text-color">{{ presented_clinic.risk_type }}</span> | ||
| <span class="nhsuk-hint">{{ presented_clinic.risk_type }}</span> |
There was a problem hiding this comment.
Suspect we want nhsuk-u-secondary-text-colour. It looks like a breaking change in v10 was to change from color to colour and we missed this. We should check if there are other classes using color and see if they need updating too.
Looks It should have identical result but is the more semantic and correct name. The hint class is meant for hint items which this isn't really.
4 participants
Description
Minor updates to the /clinics page that have been found in a snagging session referencing against the prototype.
Jira link
https://nhsd-jira.digital.nhs.uk/browse/DTOSS-11864
Review notes
Review checklist