Skip to content
This repository has been archived by the owner on Jul 17, 2018. It is now read-only.

Remove toolkit and template dependency #48

Merged
merged 8 commits into from
Jun 4, 2018

Conversation

kr8n3r
Copy link

@kr8n3r kr8n3r commented May 31, 2018

  • Updates layouts so they use the bollerplate template from GOV.UK Frontend
  • Implements Footer component from GOV.UK Frontend
  • Implements Header component from GOV.UK Frontend
  • Reimplements missing unbranded template things and fixes: Unbranded css/scss missing #41
  • Removes references to Frontend toolkit and Template
  • Updates server.js

{% from "./components/error-message/macro.njk" import govukErrorMessage %}
{% from "./components/error-summary/macro.njk" import govukErrorSummary %}
{% from "./components/fieldset/macro.njk" import govukFieldset %}
{% from "./components/file-upload/macro.njk" import govukFileUpload %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this list is out of date now

Copy link
Author

Choose a reason for hiding this comment

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

why we should have all.njk :)

@kr8n3r kr8n3r force-pushed the remove-toolkit-and-template-dependency branch 6 times, most recently from a0f536a to 9c46d71 Compare May 31, 2018 15:54
@kr8n3r kr8n3r changed the title WIP: Remove toolkit and template dependency Remove toolkit and template dependency May 31, 2018
@kr8n3r kr8n3r force-pushed the remove-toolkit-and-template-dependency branch 4 times, most recently from 3695ca3 to 4e68340 Compare June 1, 2018 07:08
Copy link
Contributor

@alex-ju alex-ju left a comment

Choose a reason for hiding this comment

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

A lot of good clean up in this PR 💯. I'll leave it for others to review too, but I'm happy to approve it.

I guess we don't need lib/govuk_template.html L9 in .gitignore anymore.

server.js Outdated
path.join(__dirname, '/lib/'),
path.join(__dirname, '/node_modules/@govuk-frontend/frontend/components')]
path.join(__dirname, '/node_modules/@govuk-frontend/frontend'), // template path
path.join(__dirname, '/node_modules/@govuk-frontend/frontend/components')
Copy link
Contributor

Choose a reason for hiding this comment

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

If we add only /node_modules/@govuk-frontend/frontend means we'll have to add /components to each component import. Not sure what's best.

Copy link
Author

Choose a reason for hiding this comment

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

yeah thought about that, but then in the DS the import statement says
{% from "file-upload/macro.njk" import govukFileUpload %}

if they copied that in it wouldn't work if we specify
'/node_modules/@govuk-frontend/frontend' path only

@@ -1,9 +1,13 @@
{% extends "govuk_template_unbranded.html" %}
{% extends "template_unbranded.html" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we change the extension of template_unbranded.html to .njk since it has Nunjucks syntax inside

@kr8n3r kr8n3r force-pushed the remove-toolkit-and-template-dependency branch 2 times, most recently from a5ea818 to bbd060c Compare June 1, 2018 17:32
@kr8n3r
Copy link
Author

kr8n3r commented Jun 1, 2018

updated with comments

@kr8n3r kr8n3r force-pushed the remove-toolkit-and-template-dependency branch 2 times, most recently from 38b349a to 7992b29 Compare June 1, 2018 17:37
@kr8n3r kr8n3r force-pushed the remove-toolkit-and-template-dependency branch from 7992b29 to 074dee2 Compare June 4, 2018 10:00
@kr8n3r kr8n3r force-pushed the remove-toolkit-and-template-dependency branch from 074dee2 to 19cbd4f Compare June 4, 2018 10:01
@kr8n3r kr8n3r merged commit 7a4df67 into master Jun 4, 2018
@kr8n3r kr8n3r deleted the remove-toolkit-and-template-dependency branch June 4, 2018 10:03
@kr8n3r kr8n3r mentioned this pull request Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unbranded css/scss missing
3 participants