-
Notifications
You must be signed in to change notification settings - Fork 17
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
Feature/add hods theme #412
base: master
Are you sure you want to change the base?
Feature/add hods theme #412
Conversation
@@ -15,4 +16,4 @@ coverage | |||
.nyc_output | |||
build/Release | |||
node_modules | |||
.emails | |||
.emails |
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.
new line? you can add this in your editor to prevent this happening in the future
if (e.code !== 'ENOENT') { | ||
throw e; | ||
} else { | ||
console.log(`${chalk.yellow('warning')}: no fonts directory found at ${config.fonts.src}`); |
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 use of chalk 👍
.then(() => spawn('cp', ['-r', config.fonts.src, config.fonts.out])) | ||
.catch(e => { | ||
if (e.code !== 'ENOENT') { | ||
throw e; |
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'm trying to understand this, if you catch an error that doesn't have a specific path/ file directory, you'll throw an error? Would it be better to provide a helpful error message?
@@ -0,0 +1,14 @@ | |||
{ | |||
"name": "hmpo-govuk-template-example", | |||
"description": "Example app for hmpo-govuk-template module", |
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.
Why does it say hmpo?
</div> | ||
{{/main}} | ||
|
||
{{/ hods-template}} |
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.
new line
/* eslint-disable max-len, no-var, vars-on-top, no-undef */ | ||
'use strict'; | ||
|
||
// TODO: update package.json(s) |
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.
what do you need to update in package.json(s) would it be best to do as this PR?
}); | ||
} | ||
} | ||
}).call(this); |
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'm guessing this is just pull it out from hods? if it is, then that's fine if we test it and it works
|
||
.hmpo { | ||
white-space: nowrap; | ||
} |
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.
Missing new line
I see that the styles are being copied across rather than having an npm module. I had a look at the HODs site and it looks like that's the way to do it. I sent a message to the team
|
|
||
.hmpo { | ||
white-space: nowrap; | ||
} |
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.
it's a shame you have to copy the sass and there is no npm module for the sass
.govuk-back-link:link, | ||
.govuk-back-link:visited { | ||
color: #005ea5 !important; | ||
} |
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.
newline
} | ||
|
||
} | ||
|
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.
are all the empty lines on purpose?
The email you have sent to ***@***.*** is for a legacy domain and will shortly be removed.
Please update your address book and send to the same address but having removed .gsi. from the domain part.
Mail to legacy addresses will stop working after 31st March 2024
|
This is really great work @andrewgibson-ibm there's just some minor linting things to sort out. I also have a question about the sass. Am I right in thinking it has been copied and pasted from the HO design system? If that's the case, then this will make it difficult to maintain in the future. For example if the HO design system updates and changes the sass how can we tell what has changed and how will we update it in the future. I don't know the answer to this so it would be good to discuss. Once we can come to a way forward with this, the next bit would be to publish a beta package of this and test it on multiple services. |
Description
Current HOF framework enables the use of
govUK
theme which implements the components and styling designed in the Government Design System. This update enables the use of the Home Office Design System by implementing the relevant styles and page template.Changes
TaskList
component has been added{{pageTitle}} - {{appTitle}}
Testing
hof
package in your apphof.settings.json
, set"theme": "hods",
npm i
, thennpm run start
to start your app