-
Notifications
You must be signed in to change notification settings - Fork 1
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.
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
[CRIMAPP-1553] replace MoJ task list with Govuk one #1299
Conversation
5be2c21
to
7702a40
Compare
7702a40
to
e8ed131
Compare
|
||
def all_tasks | ||
@all_tasks ||= map(&:items).flatten | ||
all_tasks.reject { |task| task.status.not_applicable? } | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've refactored the section renderer into two distinct parts: a TaskLit::Section (retaining the same class name as the old renderer) and a new TaskList::SectionComponent.
This change was driven by the lack of dedicated models for the TaskList classes. These classes currently encapsulate significant business logic, and could be used elsewhere. I believe moving towards dedicated models for Section, Task etc will improve things.
I've not moved them from the presenter folder, but I think we could. For now I've just separated business logic from view-related concerns.
TaskStatus::IN_PROGRESS => 'light-blue', | ||
TaskStatus::NOT_STARTED => 'blue', | ||
TaskStatus::UNREACHABLE => nil, | ||
TaskStatus::NOT_APPLICABLE => nil | ||
}.freeze | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See for https://design-system.service.gov.uk/patterns/complete-multiple-tasks/ for reasoning for changing the colours here:
Once the user has completed the task, the status should show as ‘Completed’ and be black text with no background colour. This will draw more attention to tasks that require action.
e8ed131
to
5788b6a
Compare
tag.strong id: tag_id, class: tag_classes do | ||
t!("tasklist.status.#{status}") | ||
end | ||
def to_hash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a render, this constructs arguments that can be used by
https://github.com/x-govuk/govuk-components/blob/main/app/components/govuk_component/task_list_component/status_component.rb
@@ -10,6 +10,10 @@ def initialize(crime_application:) | |||
@crime_application = crime_application | |||
end | |||
|
|||
def name |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved name here so that we can use the task subclass rather than a sparate task presenter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice refactor!
5788b6a
to
f6b026c
Compare
Description of change
Move to latest GDS task list to address accessibility issues and improve identification of unfinished tasks.
Link to relevant ticket
CRIMAPP-1553
Notes for reviewer
Screenshots of changes (if applicable)
Before changes:
After changes:
How to manually test the feature