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

SDT-644 design pattern library #822

Merged
merged 22 commits into from
Oct 17, 2017
Merged

Conversation

rpowis
Copy link
Contributor

@rpowis rpowis commented Oct 13, 2017

This PR builds a Design Systerm (based on the GOV.UK Design System) from our components and patterns.

Problem

Our component library isn't aligned with GOV.UK and has a lot of cruft in there that should be deprecated.

Example Screenshot

image

Solution

Generate a new Design System that only consists of the components and patterns that adhere to the new directory structure.

Example Screenshot

image

@rpowis
Copy link
Contributor Author

rpowis commented Oct 13, 2017

This PR relies on an update to the component-library-template. I'll add the link here when it's open.

@rpowis rpowis force-pushed the SDT-644-design-pattern-library branch from badbb76 to 5b46cd3 Compare October 13, 2017 17:09
@rpowis rpowis changed the title Sdt 644 design pattern library SDT-644 design pattern library Oct 16, 2017
@rpowis
Copy link
Contributor Author

rpowis commented Oct 16, 2017

Here's the PR for the template hmrc/component-library-template#37

@rpowis rpowis force-pushed the SDT-644-design-pattern-library branch from 5b46cd3 to 5cbd64b Compare October 16, 2017 08:25
@@ -1,7 +1,7 @@
var test = require('tape')
var prepareAssetPaths = require('../util/component-library/prepareAssetPaths')

test('prepareAssetPaths - Creates links from an array of css paths', function (t) {
test.skip('prepareAssetPaths - Creates links from an array of css paths', function (t) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these skipped?

Copy link
Contributor Author

@rpowis rpowis Oct 16, 2017

Choose a reason for hiding this comment

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

Good catch. prepareAssetPaths doesn't exist anymore so we can scrap these tests altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does exist, my bad. It's part of the component library, not the design pattern library

var mkdirp = require('mkdirp')

var writeFiles = function (config, files) {
if (!config.dest) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we check whether config is defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. Good point

throw new Error('Handlebars helpers directory must be a string or an array.')
}

if (typeof helpersDirectoryPaths === 'string') {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the isString variable

})
})
})
.then((stuff) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about the variable name :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha. Good point 😄


// gulp.task('pattern-library', ['clean-pattern-library', 'sass', 'images', 'browserify'], function () {
gulp.task('pattern-library', ['clean-pattern-library'], function () {
var env = global.runmode
Copy link
Contributor

Choose a reason for hiding this comment

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

Is runmode always defined?

)
})

compileTemplate(existingTemplate)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about this test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was to make sure that we're returning the result of Handlebars.compile without coupling the test to Handlebars, but maybe we should just mock the dependency instead.

var path = require('path')

var addFileType = function (config, files) {
if (!Array.isArray(config.src)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have config at this point for sure? if so should we rely on a previous step to verify?

@rpowis rpowis force-pushed the SDT-644-design-pattern-library branch 2 times, most recently from a8d65da to 28a4760 Compare October 16, 2017 14:51
@gavinwye gavinwye merged commit fe5de19 into master Oct 17, 2017
@gavinwye gavinwye deleted the SDT-644-design-pattern-library branch October 17, 2017 10:42
hmrc-web-operations pushed a commit that referenced this pull request Oct 17, 2017
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.

3 participants