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

Bring in new toolkit with new input fields #450

Merged
merged 6 commits into from
Feb 23, 2017

Conversation

pcraig3
Copy link
Contributor

@pcraig3 pcraig3 commented Feb 22, 2017

This pull request

  • updates the govuk_frontend toolkit to the latest
  • updates the digitalmarketplace frontend toolkit to the latest
  • updates the homepage beta header colours (blue text on a white background)
  • treats links with a role="button" as buttons (ie, "Apply for this opportunity" link)
  • removes some unused classnames
  • brings the forgotten password link closer to the password input field on the login screen

💥 Boom 💥

@pcraig3 pcraig3 force-pushed the pc-try-terrific-new-toolkit branch from 582f413 to 9e53c70 Compare February 22, 2017 12:25
@pcraig3 pcraig3 changed the title [WIP] Bring in new toolkit with new input fields Bring in new toolkit with new input fields Feb 22, 2017
@pcraig3 pcraig3 force-pushed the pc-try-terrific-new-toolkit branch from 9e53c70 to a6215dd Compare February 22, 2017 12:28
@tombye
Copy link
Contributor

tombye commented Feb 23, 2017

Excellent commits, really easy to read and understand.

👍

Brings in the latest govuk_frontend_toolkit available
on NPM, and the latest digitalmarketplace toolkit.

Also:
- removes external link styles, as these will no longer compile
- removes custom focused/selected classes for radios/checks
The govuk_frontend_toolkit stuff uses different javascript
formatting than us where they begin their files with
semicolons but don't end files with them.

Our JS does the opposite, and so concatenating the files
results in a compilation bug somewhere in the middle.
The phase banner is now always dark blue, which
doesn't work on our homepage.
Reversing the colours (white background, blue text) instead.
Javascript will automatically treat links with a
`role="button"` and a `.link-button` class as buttons.

This is part of the govuk_frontend_toolkit, not the
digitalmarketplace one.
The digitalmarketplace toolkit is no longer using this
class, so let's get rid of it.
Our secondary action links have a few context dependent
styles, so I've added a new one and I've pulled them
into their own file.

Also removed a duplicated import.
@pcraig3 pcraig3 force-pushed the pc-try-terrific-new-toolkit branch from a6215dd to 9292dd5 Compare February 23, 2017 10:11
@pcraig3 pcraig3 merged commit e8addb0 into master Feb 23, 2017
@pcraig3 pcraig3 deleted the pc-try-terrific-new-toolkit branch February 23, 2017 13:29
pcraig3 added a commit that referenced this pull request Feb 24, 2017
Reverts #452

So yesterday, we merged in these changes (#450) and then our functional tests started to break.
Upon investigation, we found that it was not able to find checkbox/radio elements [when calling its default 'find_all' method and looking for input fields](https://github.com/alphagov/digitalmarketplace-functional-tests/blob/19f5fa5c7aa2db20dddb921e96869a06614864ac/features/support/form_helpers.rb#L52).

Since the default input elements are really small, the new elements actually hide the inputs and draw a custom interface using the `::before` and `::after` psuedo-elements. @robinwhittleton talks about how [hiding the elements was an accessibility challenge](https://gdstechnology.blog.gov.uk/2016/12/13/improving-forms-with-new-radios-and-checkboxes/), but now they work with real-world accessibility software.

Unfortunately, [selenium doesn't find hidden elements](http://stackoverflow.com/questions/22110282/how-to-click-on-hidden-element-in-selenium-webdriver), which means it can't find our inputs.

Since we can't make large interface changes while G-Cloud is open for suppliers, but we can make testing changes, the decision has been made to merge this now and fix the tests later.
pcraig3 added a commit that referenced this pull request Feb 28, 2017
Reverts #452

So yesterday, we merged in these changes (#450) and then our functional tests started to break.
Upon investigation, we found that it was not able to find checkbox/radio elements [when calling its default 'find_all' method and looking for input fields](https://github.com/alphagov/digitalmarketplace-functional-tests/blob/19f5fa5c7aa2db20dddb921e96869a06614864ac/features/support/form_helpers.rb#L52).

Since the default input elements are really small, the new elements actually hide the inputs and draw a custom interface using the `::before` and `::after` psuedo-elements. @robinwhittleton talks about how [hiding the elements was an accessibility challenge](https://gdstechnology.blog.gov.uk/2016/12/13/improving-forms-with-new-radios-and-checkboxes/), but now they work with real-world accessibility software.

Unfortunately, [selenium doesn't find hidden elements](http://stackoverflow.com/questions/22110282/how-to-click-on-hidden-element-in-selenium-webdriver), which means it can't find our inputs.

Since we can't make large interface changes while G-Cloud is open for suppliers, but we can make testing changes, the decision has been made to merge this now and fix the tests later.
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.

2 participants