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

Namespace nunjucks and components #1458

Merged
merged 6 commits into from
Jun 21, 2019
Merged

Namespace nunjucks and components #1458

merged 6 commits into from
Jun 21, 2019

Conversation

aliuk2012
Copy link
Contributor

@aliuk2012 aliuk2012 commented Jun 17, 2019

To namespace GOV.UK Frontend components, a new govuk directory has been added to the package and src directories. This means that users should now be able to run GOV.UK Frontend along side their own departmental frontend without any conflicts. There was no need to add govuk directory to dist or public as they would only contain compiled code and be used differently from package and src

Changes to src directory

Before
src
├── assets
├── components
├── core
├── helpers
├── objects
├── overrides
├── settings
├── tools
├── utilities
├── vendor
├── README.md
├── all-ie8.scss
├── all.js
├── all.scss
├── all.test.js
├── common.js
├── template.njk
└── template.test.js
After
src
└── govuk
    ├── assets
    ├── components
    ├── core
    ├── helpers
    ├── objects
    ├── overrides
    ├── settings
    ├── tools
    ├── utilities
    ├── vendor
    ├── README.md
    ├── all-ie8.scss
    ├── all.js
    ├── all.scss
    ├── all.test.js
    ├── common.js
    ├── template.njk
    └── template.test.js

Changes to package

Before
package
├── assets
├── components
├── core
├── helpers
├── objects
├── overrides
├── settings
├── tools
├── utilities
├── vendor
├── README.md
├── all-ie8.scss
├── all.js
├── all.scss
├── common.js
├── govuk-prototype-kit.config.json
├── package.json
└── template.njk
After
package
├── govuk
│   ├── assets
│   ├── components
│   ├── core
│   ├── helpers
│   ├── objects
│   ├── overrides
│   ├── settings
│   ├── tools
│   ├── utilities
│   ├── vendor
│   ├── all-ie8.scss
│   ├── all.js
│   ├── all.scss
│   ├── common.js
│   └── template.njk
├── README.md
├── govuk-prototype-kit.config.json
└── package.json

Tested:

  • npm start still works
  • npm run pre-release still works
  • npm run release-to-branch still works
  • check no changes to dist and public directories.
Details

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 10:34 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 10:41 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 12:23 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 12:25 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 12:27 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 12:31 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 12:34 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 17, 2019 14:10 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 18, 2019 13:40 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 18, 2019 13:44 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 18, 2019 16:10 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 08:24 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 08:45 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 08:56 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 09:34 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 09:42 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 10:02 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 10:20 Inactive
@aliuk2012 aliuk2012 marked this pull request as ready for review June 19, 2019 10:20
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Looks great 🌮 Left a couple of non-blocking comments.

There are quite a few places in the documentation where we need to reflect this change (do a search for "src/" in docs/ to see them).

tasks/gulp/compile-assets.js Outdated Show resolved Hide resolved
tasks/gulp/compile-assets.js Outdated Show resolved Hide resolved
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1458 June 19, 2019 11:06 Inactive
@aliuk2012 aliuk2012 added this to the v3.0.0 milestone Jun 19, 2019
@aliuk2012
Copy link
Contributor Author

I'll do a separate PR for docs and assets as this PR is already a large one with all the renames

@36degrees
Copy link
Contributor

Generally looking really good. The before and after directory structures in the first commit message use different depths (one includes all files; the other just includes the top-level folder names) – this makes it quite difficult to see the difference. Could we re-word the commit message using consistent directory structures?

aliuk2012 and others added 6 commits June 19, 2019 16:05
Due to a known Github bug (isaacs/github#900)
you can’t view previous history after using `git mv`. To view the previous
history of files in src/govuk, use git blame.
The URL format is repo-name/blame/branch/dir/file.
For example https://github.com/alphagov/govuk-frontend/blame/master/src/README.md

Description of work:
This is a start to namespacing our code under govuk and has been accepted
by the community.

Directory structure before the commit:

src
├── assets
├── components
├── core
├── helpers
├── objects
├── overrides
├── settings
├── tools
├── utilities
├── vendor
├── README.md
├── all-ie8.scss
├── all.js
├── all.scss
├── all.test.js
├── common.js
├── template.njk
└── template.test.js

Directory structure after this commit:

src
└── govuk
    ├── assets
    ├── components
    ├── core
    ├── helpers
    ├── objects
    ├── overrides
    ├── settings
    ├── tools
    ├── utilities
    ├── vendor
    ├── README.md
    ├── all-ie8.scss
    ├── all.js
    ├── all.scss
    ├── all.test.js
    ├── common.js
    ├── template.njk
    └── template.test.js

See proposal for more detail
https://github.com/alphagov/govuk-design-system-architecture/blob/master/proposals/005-namespace-govuk-frontend-nunjucks-sass-using-a-nested-folder-structure.md

Co-authored-by: Hanna Laakso <hanna.laakso@digital.cabinet-office.gov.uk>
Co-authored-by: Hanna Laakso <hanna.laakso@digital.cabinet-office.gov.uk>
Before:

package
├── assets
├── components
├── core
├── helpers
├── objects
├── overrides
├── settings
├── tools
├── utilities
├── vendor
├── README.md
├── all-ie8.scss
├── all.js
├── all.scss
├── common.js
├── govuk-prototype-kit.config.json
├── package.json
└── template.njk

After:

package
├── govuk
│   ├── assets
│   ├── components
│   ├── core
│   ├── helpers
│   ├── objects
│   ├── overrides
│   ├── settings
│   ├── tools
│   ├── utilities
│   ├── vendor
│   ├── all-ie8.scss
│   ├── all.js
│   ├── all.scss
│   ├── common.js
│   └── template.njk
├── README.md
├── govuk-prototype-kit.config.json
└── package.json

See proposal for more detail
https://github.com/alphagov/govuk-design-system-architecture/blob/master/proposals/005-namespace-govuk-frontend-nunjucks-sass-using-a-nested-folder-structure.md
This commit fixes the issues identified while trying to run
`npm run build:dist`.

Current and desired directory structure:

dist
├── assets
│   ├── fonts
│   └── images
├── VERSION.txt
├── govuk-frontend-2.13.0.min.css
├── govuk-frontend-2.13.0.min.js
└── govuk-frontend-ie8-2.13.0.min.css

See proposal for more detail
https://github.com/alphagov/govuk-design-system-architecture/blob/master/proposals/005-namespace-govuk-frontend-nunjucks-sass-using-a-nested-folder-structure.md
Some of the changes made before meant that js files were being compiled
and placed in a `govuk` directory. This meant that the browser could not
find the js files when viewing the page.

Current & desired folder structure for `public` when serving up the review app:

public
├── components
│   ├── accordion
│   ├── button
│   ├── character-count
│   ├── checkboxes
│   ├── details
│   ├── error-summary
│   ├── header
│   ├── radios
│   └── tabs
├── vendor
│   └── polyfills
├── all.js
├── app-ie8.css
├── app-legacy-ie8.css
├── app-legacy.css
├── app.css
└── common.js
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Looks good to me 🙌 Need another person to approve who didn't do work on this

@aliuk2012 aliuk2012 merged commit 063cd8e into master Jun 21, 2019
@NickColley NickColley deleted the al/namespace branch June 21, 2019 11:52
quis added a commit to quis/focus-state-prototype that referenced this pull request Nov 14, 2019
If you try to run this prototype now it doesn’t work, because the
version of GOV.UK Frontend that you get if running
```shell
npm install govuk-frontend@^3.0.0
```

introduces a breaking change[1] that came with version 3.0.0 of GOV.UK
Frontend.

This repo was using a pre-release version of 3.0.0 in order to get
the new focus styles. However when it was updated to 3.0.0 stable it
wasn’t also updated[2] to respect the folder structure changes that came
with 3.0.0.

This commit updates the paths to the macros to point to the right place.

1. alphagov/govuk-frontend#1458
2. alphagov@22d7f9c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

5 participants