Skip to content
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

Add learn-to-drive-a-car task list page #400

Merged
merged 2 commits into from
Nov 1, 2017
Merged

Add learn-to-drive-a-car task list page #400

merged 2 commits into from
Nov 1, 2017

Conversation

sihugh
Copy link
Contributor

@sihugh sihugh commented Oct 27, 2017

This is reliant on the tasklist component in alphagov/static#1173, so to test this you will need to have a local static running on the create-tasklist-component branch until that is merged and deployed.

We will add Smokey tests to cover this behaviour similar to the tests for the accordion

This page is the first item in the public beta of task list pages run by the modelling-services team.

The content item for this page is manually published from collections-publisher

The json file recording the content is deep symbolized after parsing to facilitate working with components which use symbolized keys.

This page is expected to mostly replicate the prototype although some small differences are likely after design review of the tasklist component.

Somewhat to do with: https://trello.com/c/M8ZYKOF1/231-create-hardcoded-json-for-learning-to-drive-beta
but mostly: https://trello.com/c/nByQmGoZ/201-implement-tasklist-page-in-preparation-for-public-beta-rollout

Screenshots

Desktop

screen shot 2017-10-27 at 08 50 36

screen shot 2017-10-27 at 08 51 02

Mobile

collections dev gov uk_learn-to-drive-a-car iphone 6 1

collections dev gov uk_learn-to-drive-a-car iphone 6 2

@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 27, 2017 08:00 Inactive
@sihugh sihugh force-pushed the add-tasklist-page branch from 907e190 to d0abb7c Compare October 27, 2017 08:58
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 27, 2017 08:58 Inactive
@sihugh sihugh force-pushed the add-tasklist-page branch from d0abb7c to 7e7a5e8 Compare October 27, 2017 09:03
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 27, 2017 09:03 Inactive
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 27, 2017 09:38 Inactive
[
{
"title": "Check you're allowed to drive",
"panel": "Most people can start learning to drive when they’re 17.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can use the 'panel' option here, but I'd recommend the 'panel_descriptions' as they're styled by the component, which means if we want to change how they look we can more easily do that. Panel descriptions are an array of strings, which get inserted into P tags, i.e.

panel_descriptions: [
"This is a panel description.",
"This is also a panel description. It has been filled with words such as CONTENT and ACCORDION in order to fill the horizontal space and wrap onto a second line."
]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good shout, thanks. I'll get these switched over and update the screenshots

</div>
<div class="grid-row">
<div class="column-two-thirds">
<%= render "govuk_component/tasklist", @tasklist[:tasklist] %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@sihugh sihugh force-pushed the add-tasklist-page branch from 322ea3f to dd3ec70 Compare October 27, 2017 15:23
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 27, 2017 15:23 Inactive
@sihugh sihugh force-pushed the add-tasklist-page branch from dd3ec70 to 37e56de Compare October 27, 2017 15:27
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 27, 2017 15:27 Inactive
@@ -0,0 +1,5 @@
class TasklistController < ApplicationController
def show
@tasklist = TasklistContent.learn_to_drive_config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should follow the convention implemented in other controllers, of not using instance variables

      render :show, locals: {
        tasklist: TasklistContent.learn_to_drive_config
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed it to match, thanks

config/routes.rb Outdated
@@ -6,6 +6,8 @@
mount JasmineRails::Engine => '/specs' if defined?(JasmineRails)
mount GovukPublishingComponents::Engine, at: "/component-guide" if defined?(GovukPublishingComponents)

get "learn-to-drive-a-car", to: 'tasklist#show'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you missing / before the learn-to-drive-a-car

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good spot, thanks. It miraculously works either way though :-D

@sihugh sihugh force-pushed the add-tasklist-page branch from 37e56de to 9f164da Compare October 30, 2017 13:26
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 30, 2017 13:26 Inactive
@sihugh sihugh force-pushed the add-tasklist-page branch from 9f164da to a75d4f9 Compare October 30, 2017 13:30
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 30, 2017 13:30 Inactive
@sihugh sihugh changed the title [DO NOT MERGE] Add learn-to-drive-a-car task list page Add learn-to-drive-a-car task list page Oct 31, 2017
end

def tasklist_step_keys(tasklist_args)
tasklist_args[:steps].flatten.map(&:keys).flatten.uniq
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can use flat_map

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks

@sihugh sihugh force-pushed the add-tasklist-page branch from a75d4f9 to 2c639dd Compare October 31, 2017 14:45
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 October 31, 2017 14:45 Inactive
@sihugh sihugh force-pushed the add-tasklist-page branch from 2c639dd to 6eb23cd Compare November 1, 2017 11:23
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 November 1, 2017 11:23 Inactive
This will help catch linting errors before they are flagged up by CI.
@sihugh sihugh force-pushed the add-tasklist-page branch from 6eb23cd to fed764c Compare November 1, 2017 13:31
@tijmenb tijmenb temporarily deployed to govuk-collections-pr-400 November 1, 2017 13:32 Inactive
This page is the first item in the public beta of task list pages run by the
modelling-services team.

The content item for this page is [manually published from collections-publisher](alphagov/collections-publisher#250)

The json file recording the content is deep symbolized to facilitate working
with components which use symbolized keys.

This page is expected to replicate [the prototype](https://govuk-services-prototype.herokuapp.com/services/learn-to-drive-a-car)
@sihugh
Copy link
Contributor Author

sihugh commented Nov 1, 2017

This is now ready for merging.

@sihugh sihugh merged commit 7533dd8 into master Nov 1, 2017
@sihugh sihugh deleted the add-tasklist-page branch November 1, 2017 15:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants