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 #421

Merged
merged 1 commit into from
Jan 11, 2018

Conversation

hannalaakso
Copy link
Member

@hannalaakso hannalaakso commented Jan 10, 2018

Update with all the latest changes from source:

  • Add width classes, which were based on form-control classes that were
    specific to form control in Elements. (PR #413)
  • Make buttons 40px high including box shadow (PR #416)
  • Fix focus outline style in Chrome and Safari (PR #414)
  • Remove contributors list from template, fix template markup and update README files (PR #403)
  • Generate breadcrumb chevrons using pseudo-elements (PR #407)
  • Rename ‘govuk-body-lede’ to ‘govuk-body-lead’. (PR #405) Breaking change
  • Fix undefined classes in date input macro (PR #410)
  • Allow line height to be overridden in typography helpers (PR #404)
  • Pluralise radio component (PR #388) Breaking change
  • Pluralise checkbox component (PR #384) Breaking change
  • Add documentation for typography helpers / core, simplify syntax (PR #400)
  • Add adjacent styles for headings after lists (PR #408)

Note: this step of the publishing process is not perfect right now, it's hard to check that this contains the correct changes. We will have some tests eventually to verify.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-421 January 10, 2018 14:39 Inactive
@@ -1,31 +1,31 @@
{
"name": "@govuk-frontend/all",
"version": "0.0.19-alpha",
"version": "0.0.20-alpha",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this normally done by lerna? Do we need to do this manually?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 After consulting Lerna documentation it appears it's not necessary to change package.json manually. Have taken this out.

@hannalaakso hannalaakso force-pushed the update-publish-and-packages-10-jan-18 branch from e36cb87 to 1f7b816 Compare January 10, 2018 15:30
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-421 January 10, 2018 15:31 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.

A couple of things that need fixing, and some larger issues I'm not sure we can address right now.

@@ -5,7 +5,7 @@
"@govuk-frontend/back-link": "^0.0.19-alpha",
"@govuk-frontend/breadcrumbs": "^0.0.19-alpha",
"@govuk-frontend/button": "^0.0.19-alpha",
"@govuk-frontend/checkbox": "^0.0.19-alpha",
"@govuk-frontend/checkboxes": "^0.0.20-alpha",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this bump intentional when renaming?

Copy link
Member Author

@hannalaakso hannalaakso Jan 10, 2018

Choose a reason for hiding this comment

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

As far as npm is concerned, we've published a new package, not renamed one.

@NickColley was looking into this and added a note to dev time to discuss how we should handle removing npm packages such as govuk-frontend/checkbox

@@ -55,6 +63,21 @@
color: $govuk-text-colour;
}
}

// The following adjustments do not work for <input type="button"> as
Copy link
Contributor

Choose a reason for hiding this comment

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

So I think postcss-nested is doing crazy things here. It's removed the nesting of the ::before and ::active::before by shunting them down the file, taking the immediately preceding comment line, but leaving the rest of the comment here, divorced from its context.

Not sure what we can do about this in the short term, but… this is not great.

Copy link
Member Author

Choose a reason for hiding this comment

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

As just discussed in person, we should review why we're using postcss-nested when @igloosi is back.


background: transparent;
}
// 🎉
Copy link
Contributor

Choose a reason for hiding this comment

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

:trollface:

@@ -0,0 +1,7 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is meant to be added? None of the other components have package.json files in the src directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes indeed. I've taken it out.

@hannalaakso hannalaakso merged commit e3ff994 into master Jan 11, 2018
@hannalaakso hannalaakso deleted the update-publish-and-packages-10-jan-18 branch January 11, 2018 09:14
@hannalaakso hannalaakso restored the update-publish-and-packages-10-jan-18 branch January 11, 2018 10:43
@hannalaakso hannalaakso deleted the update-publish-and-packages-10-jan-18 branch January 11, 2018 10:47
hannalaakso added a commit that referenced this pull request Jan 11, 2018
hannalaakso added a commit that referenced this pull request Jan 11, 2018
…jan-18

Fix to #421 - restore correct FE version for radios and checkboxes
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.

4 participants