Remove the $path override from the helpers partial #292

gemmaleigh · 2016-08-24T11:46:42Z

What problem does the pull request solve?

This PR fixes image paths for GOV.UK elements, which are currently broken.

What does it do?

Pull #285 added a !default flag to the $path variable, as $path is already set as false !default in url helpers, the file-url function fell through to use image-url.
This resulted in broken images for GOV.UK elements.

How has this been tested?

I think the simplest solution here is to not try and define $path in the govuk-elements-sass files, instead this should be defined in the main.scss file for each project.

Moving the $path variable to main.scss for govuk elements fixes this. It will however, break background images for anyone using the govuk-elements-sass files, they will need to define $path for their own project.

Although it is a breaking change, I think it's a sensible one.

cc. @gavboulton can you let me know if this solution works for you, as you introduced this change?

cc. @robinwhittleton.

This will need documentation to make it clear that $path must be defined on a project by project basis. Digital marketplace already do this here.

Screenshots (if appropriate):

Broken background images before:

Working background images after:

What type of change is it?

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Put an `x` in all the boxes that apply

My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.

url-helpers from the frontend toolkit already allows $path to be overridden. This will require $path to be defined on a project-by-project basis, before govuk-elements is imported. main.scss has an example of this.

gavboulton · 2016-08-24T13:25:06Z

This looks good to me.

Sorry everyone for breaking your things!

gemmaleigh · 2016-08-24T15:39:46Z

@gavboulton no worries! The override in helpers.scss was just confusing matters, it's much simpler if it works in this way.

Add a ‘Usage’ section to the README with details of how to set a path for image assets (if not using Rails or Compass).

- Remove the images path override from the helpers partial ([PR #292](#292)). This will break background images that are currently using the `file-url` function from [the url-helpers partial in the frontend toolkit](https://github.com/alphagov/govuk_frontend_toolkit/blob/d54c9b26d314a6e6cb50ba90f6e96ca50049498f/stylesheets/_url-helpers.scss). As `$path` will vary from project to project you will need to define this in your main application stylesheet. - Bump govuk frontend toolkit to 4.16.1 ([PR #288](#288)). This adds space key activation to links with a role of button. - Form validation patterns for conditionally revealing content ([PR #286](#286)) - Centre text on full-width buttons ([PR #289](#289)) - Lint JS code using [StandardJS](http://standardjs.com/) ([PR #290](#290)) *This version deprecates external link styles. If your service has user research that indicates that external links are useful (or not) then we’d like to hear from you either on Slack, [digital-service-designers](https://groups.google.com/a/digital.cabinet-office.gov.uk/forum/#!forum/digital-service-designers) or [opening an issue](https://github.com/alphagov/govuk_elements/issues/new).*

# 2.0.0 - Remove the images path override from the helpers partial ([PR #292](alphagov/govuk_elements#292)). This will break background images that are currently using the `file-url` function from [the url-helpers partial in the frontend toolkit](https://github.com/alphagov/govuk_frontend_toolkit/blob/d54c9b2 6d314a6e6cb50ba90f6e96ca50049498f/stylesheets/_url-helpers.scss). As `$path` will vary from project to project you will need to define this in your main application stylesheet. - Bump govuk frontend toolkit to 4.16.1 ([PR #288](alphagov/govuk_elements#288)). This adds space key activation to links with a role of button. - Form validation patterns for conditionally revealing content ([PR #286](alphagov/govuk_elements#286)) - Centre text on full-width buttons ([PR #289](alphagov/govuk_elements#289)) - Lint JS code using [StandardJS](http://standardjs.com/) ([PR #290](alphagov/govuk_elements#290)) *This version deprecates external link styles. If your service has user research that indicates that external links are useful (or not) then we’d like to hear from you either on Slack, [digital-service-designers](https://groups.google.com/a/digital.cabinet- office.gov.uk/forum/#!forum/digital-service-designers) or [opening an issue](https://github.com/alphagov/govuk_elements/issues/new).* # 1.2.2 - Bump govuk frontend toolkit to 4.14.4 (PR #278) - Add a .column-full class (PR #270) - Add a file upload example (PR #268) - Remove the blue outline from the main content area (PR #265) - Fix legend text wrapping in IE (PR #248) # 1.2.1 - Consistent spacing underneath block labels and toggled content (PR #229) - Remove underlines from abbreviations in Firefox (PR #241)

- made some minor adjustments to form to look good with new radio/checkbox styles - removed field_with_errors wrapping div which conflicts with new radio/checkbox JS modules Full list of changes: Remove the images path override from the helpers partial alphagov/govuk_elements#292 Bump govuk frontend toolkit to 4.16.1 alphagov/govuk_elements#288 Form validation patterns for conditionally revealing content alphagov/govuk_elements#286 Centre text on full-width buttons alphagov/govuk_elements#289 Lint JS code using StandardJS alphagov/govuk_elements#290 Update govuk_frontend_toolkit to 5.0.0 and govuk_template to 0.19.0 alphagov/govuk_elements#333 Add a select box example alphagov/govuk_elements#303 Add bullet points describing use of disabled buttons alphagov/govuk_elements#304 Add reasoning for native over custom file inputs Clear floats on details elements alphagov/govuk_elements#328 Add a .bold utility class alphagov/govuk_elements#321 Remove external link styles alphagov/govuk_elements#274 Remove rounded corners on form inputs elements and textareas in iOS alphagov/govuk_elements#342 Fix focus outline on form fields in Chrome / Safari alphagov/govuk_elements#346 Updates the contribution guidelines alphagov/govuk_elements#339 Recommend .visually-hidden over .visuallyhidden alphagov/govuk_elements#341 Add snippets for data visualisation examples alphagov/govuk_elements#350 alphagov/govuk_elements#351 Fix the grey left hand border example for Radios and checkboxes alphagov/govuk_elements#349 Fix the spacing underneath the "inline" block label example alphagov/govuk_elements#348 Custom radios / checkboxes alphagov/govuk_elements#296 Fix contrast issues with phase banners. Use $govuk-blue for the phase tag and update the examples alphagov/govuk_elements#364

- made some minor adjustments to form to look good with new radio/checkbox styles Full list of changes: Remove the images path override from the helpers partial alphagov/govuk_elements#292 Bump govuk frontend toolkit to 4.16.1 alphagov/govuk_elements#288 Form validation patterns for conditionally revealing content alphagov/govuk_elements#286 Centre text on full-width buttons alphagov/govuk_elements#289 Lint JS code using StandardJS alphagov/govuk_elements#290 Update govuk_frontend_toolkit to 5.0.0 and govuk_template to 0.19.0 alphagov/govuk_elements#333 Add a select box example alphagov/govuk_elements#303 Add bullet points describing use of disabled buttons alphagov/govuk_elements#304 Add reasoning for native over custom file inputs Clear floats on details elements alphagov/govuk_elements#328 Add a .bold utility class alphagov/govuk_elements#321 Remove external link styles alphagov/govuk_elements#274 Remove rounded corners on form inputs elements and textareas in iOS alphagov/govuk_elements#342 Fix focus outline on form fields in Chrome / Safari alphagov/govuk_elements#346 Updates the contribution guidelines alphagov/govuk_elements#339 Recommend .visually-hidden over .visuallyhidden alphagov/govuk_elements#341 Add snippets for data visualisation examples alphagov/govuk_elements#350 alphagov/govuk_elements#351 Fix the grey left hand border example for Radios and checkboxes alphagov/govuk_elements#349 Fix the spacing underneath the "inline" block label example alphagov/govuk_elements#348 Custom radios / checkboxes alphagov/govuk_elements#296 Fix contrast issues with phase banners. Use $govuk-blue for the phase tag and update the examples alphagov/govuk_elements#364

Remove the $path override from helpers

fb11c70

url-helpers from the frontend toolkit already allows $path to be overridden. This will require $path to be defined on a project-by-project basis, before govuk-elements is imported. main.scss has an example of this.

Update the README, add $path requirement

fdf976f

Add a ‘Usage’ section to the README with details of how to set a path for image assets (if not using Rails or Compass).

gemmaleigh force-pushed the fix-image-paths branch from 17813b2 to fdf976f Compare August 24, 2016 15:46

gemmaleigh merged commit 51c8182 into master Aug 24, 2016

robinwhittleton mentioned this pull request Aug 25, 2016

Release v2.0.0 #293

Merged

gemmaleigh deleted the fix-image-paths branch September 12, 2016 10:05

Guntrisoft mentioned this pull request Dec 2, 2016

Update govuk_elements to version 2.2.1 guidance-guarantee-programme/pension_guidance#701

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove the $path override from the helpers partial #292

Remove the $path override from the helpers partial #292

gemmaleigh commented Aug 24, 2016 •

edited

Loading

gavboulton commented Aug 24, 2016

gemmaleigh commented Aug 24, 2016

Remove the $path override from the helpers partial #292

Remove the $path override from the helpers partial #292

Conversation

gemmaleigh commented Aug 24, 2016 • edited Loading

What problem does the pull request solve?

What does it do?

How has this been tested?

Screenshots (if appropriate):

What type of change is it?

Put an x in all the boxes that apply

gavboulton commented Aug 24, 2016

gemmaleigh commented Aug 24, 2016

gemmaleigh commented Aug 24, 2016 •

edited

Loading

Put an `x` in all the boxes that apply