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

Chore(packages): copy changes to packages #437

Merged
merged 3 commits into from
Jan 18, 2018

Conversation

hannalaakso
Copy link
Member

Update with all the latest changes from source:

Breaking changes:

  • The link styles in the core layer no longer style a elements directly, but
    instead provide a govuk-link class which you will need to apply to links
    individually. (PR #427)
  • The link component has been removed from Frontend as the link styles have
    been moved to the core
    (PR #431)
  • Rename legal-text argument accepted by legal-text component to text (PR #431)
  • Rename legal-text component to warning-text (PR #431)

New features:

  • The prose scope has been extended to style links, which means links within the
    scope do not need the govuk-link class applied.
    (PR #427)
  • The muted link variant from the link component is now available as a core
    class (govuk-link--muted).
    (PR #427)

Fixes:

  • The error summary component allows users to pass HTML for an entry in the list
    again. (PR #428)
  • Error list entries in the error summary component no longer get wrapped in
    links when no href is provided.
    (PR #428)
  • Remove redundant 'resets' for link print styles
    (PR #427)
  • The back link, breadcrumbs, error summary, previous / next and skip link
    components have been updated to include explicit link styling, as they
    previously relied on the global link styles.
    (PR #427)
  • Adjust warning-text icon by 1px for New Transport

Internal:

  • Add prose scope example (PR #429)
  • Links within the review app and the examples have been updated to use the
    govuk-link class.
    (PR #427)
  • Improve documentation around publishing (PR #430)
  • Improve documentation around contributing (PR #433)

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-437 January 18, 2018 11:54 Inactive
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

The changes themselves look fine but there's something weird going on with additional files being added, including parts of the sass-mq library?

@@ -0,0 +1 @@
../../../back-link
Copy link
Contributor

Choose a reason for hiding this comment

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

Where did these files come from? Are these meant to be here?

@@ -0,0 +1 @@
../../../globals
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, not sure where this came from.

@@ -0,0 +1,19 @@
Copyright (c) 2013-2016 Guardian Media Group and contributors
Copy link
Contributor

Choose a reason for hiding this comment

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

This shouldn't be here.

alex-ju
alex-ju previously approved these changes Jan 18, 2018
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.

Looks good, but needs conflict to be resolved first.

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Need to address the issues above before this can be merged.

 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
 - @govuk-frontend/[email protected]
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-437 January 18, 2018 12:04 Inactive
@alex-ju alex-ju dismissed their stale review January 18, 2018 12:06

Got a better review

@joelanman
Copy link
Contributor

the changelog above doesnt seem to have the govuk-button-hover-colour PR which is in the main changelog

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-437 January 18, 2018 12:10 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-437 January 18, 2018 12:14 Inactive
@hannalaakso
Copy link
Member Author

hannalaakso commented Jan 18, 2018

Thanks @36degrees. I've got no explanation for those blank files inside packages/all or the sass-mq licence files, I've removed them. I'll note this in the docs somehow as something to look out for.

@joelanman That would be because when I rebased master into this branch, it took your CHANGELOG message from master under Unreleased and moved it from into 0.0.22-alpha (Breaking release) as I renamed the Unreleased into 0.0.22-alpha (Breaking release) in my commit. I'll mention this in the docs too. To clarify, the govuk-button-hover-colour PR wasn't part of this release.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-437 January 18, 2018 12:23 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-437 January 18, 2018 12:29 Inactive
@alex-ju
Copy link
Contributor

alex-ju commented Jan 18, 2018

Reviewed again 👍

@36degrees
Copy link
Contributor

For posterity – there were originally a number of symlinks accidentally introduced as part of this PR (e.g. packages/back-link/node_modules/@govuk-frontend/globals which symlinked to ../../../globals). These were added when the Frontend packages folder was symlinked into the Design System and npm install was run, and accidentally committed.

However, this was done after the packages were released to npm as part of the verification process, and we have checked that the releases on npm are fine. As such, we've edited the offending commit to negate the addition of the symlinks (and a spurious .DS_Store).

We also discussed the way that commits were done – with the changes to the packages folder being committed after the commit to bump the versions and release – and decided that it made more sense to commit the changes first. So there will be a further PR to update the docs along with the other changes that Hanna has been working on.

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Happy with this now. Although I helped with the fixes, @alex-ju has also reviewed so this can be merged.

@hannalaakso hannalaakso merged commit 0091fc8 into master Jan 18, 2018
@hannalaakso hannalaakso deleted the frontend-release-0.0.22 branch January 18, 2018 15:31
@hannalaakso hannalaakso changed the title Frontend release 0.0.22 Chore(packages): copy changes to packages Jan 18, 2018
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.

5 participants