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

Add input prefix and suffix #1816

Merged
merged 9 commits into from
Sep 7, 2020
Merged

Add input prefix and suffix #1816

merged 9 commits into from
Sep 7, 2020

Conversation

simonwhatley
Copy link
Contributor

@simonwhatley simonwhatley commented May 26, 2020

What

Adds prefix and suffix elements to the input component.

Why

A number of local and departmental design systems (GOV.UK Publishing Components, HMRC, MOJ and ONS) are using prefixes and suffixes to indicate currencies and measurements.

Examples

Prefix

HTML

<div class="govuk-form-group">
  <label class="govuk-label" for="input-with-prefix-attribute">
    Amount
  </label>
  <div class="govuk-input__wrapper">
    <span class="govuk-input__prefix" aria-hidden="true">£</span>
    <input class="govuk-input" id="input-with-prefix-attribute" name="amount" type="number">
  </div>
</div>

Macro

{% from "input/macro.njk" import govukInput %}

{{ govukInput({
  "label": {
    "text": "Amount"
  },
  "id": "input-with-prefix-attribute",
  "name": "amount",
  "type": "number",
  "prefix": {
    "text": "£"
  }
}) }}

image

Suffix

HTML

<div class="govuk-form-group">
  <label class="govuk-label" for="input-with-suffix-attribute">
    Weight
  </label>
  <div class="govuk-input__wrapper">
    <input class="govuk-input" id="input-with-suffix-attribute" name="weight" type="number">
    <span class="govuk-input__suffix" aria-hidden="true">kg</span>
  </div>
</div>

Macro

{% from "input/macro.njk" import govukInput %}

{{ govukInput({
  "label": {
    "text": "Weight"
  },
  "id": "input-with-suffix-attribute",
  "name": "weight",
  "type": "number",
  "suffix": {
    "text": "kg"
  }
}) }}

image

Fixes #1915

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 May 27, 2020 14:31 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 May 28, 2020 15:00 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 May 29, 2020 13:25 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 July 16, 2020 18:35 Inactive
@hannalaakso
Copy link
Member

This work was reviewed by the Working Group in June, see alphagov/govuk-design-system#1269 (comment) for their comments.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 August 27, 2020 14:40 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 August 27, 2020 17:03 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 September 1, 2020 10:12 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 September 1, 2020 10:32 Inactive
@hannalaakso
Copy link
Member

hannalaakso commented Sep 1, 2020

I've implemented the design for very narrow viewports. It only kicks in under 320px so I think that it's most likely to be seen by users who zoom in on a mobile device. I've tested this using the 'Aa' zoom button on iOS Safari:

Screenshot 2020-09-01 at 10 45 22

Screenshot 2020-09-01 at 10 45 59

Screenshot 2020-09-01 at 10 46 41

Screenshot 2020-09-01 at 11 09 25

Screenshot 2020-09-01 at 10 47 44

Screenshot 2020-09-01 at 11 35 27

I've also tried to increase text scaling on Android Chrome but it mainly seems to work for text like paragraphs and headings, it doesn't do anything for form elements for instance. I'll raise a separate issue to investigate that.

Tested on
✅ IE8 (See commit for details)
❓ IE9 (Doesn't support flex so there's a small gap between elements and some borders are incorrectly removed. However the number of IE9 users is abysmally small so I'm not worried about it)
✅ IE10
✅ IE11
✅ Windows 10 Edge 84
✅ Android 10 - Samsung galaxy S20: FF, Chrome, Samsung Internet
✅ Android 6 - Galaxy S7: FF, Chrome
✅ iOS 13 - iPhone XS: Safari
✅ iOS 11 - iPhone 6S: Safari and FF
✅ iOS 9 - iPhone 6S: Safari and FF
❓ iOS 8 - iPhone 6S: Safari and FF (Things aren't quite correctly aligned, probably a flex issue)
❌ iOS 7 - iPhone 5: Safari (Things aren't quite correctly aligned and the suffix drops onto next line - but this is way outside our support matrix and the component is still usable)
✅ MacOS Safari 13.1
✅ MacOS Firefox 80.0 with inverted colours
✅ MacOS Chrome 84
✅ Windows 10 Firefox 80.0
✅ Windows 10 Edge 84


.govuk-input:focus {
// Hack to stop focus style being overlapped by the suffix
z-index: 1;
Copy link
Member

@hannalaakso hannalaakso Sep 1, 2020

Choose a reason for hiding this comment

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

I wish we had another way of fixing this but I can't find anything else to stop the suffix overlapping the input focus border.

@hannalaakso hannalaakso marked this pull request as ready for review September 1, 2020 12:22
src/govuk/components/input/_index.scss Outdated Show resolved Hide resolved
src/govuk/components/input/template.njk Outdated Show resolved Hide resolved
src/govuk/components/input/input.yaml Outdated Show resolved Hide resolved
src/govuk/components/input/template.test.js Show resolved Hide resolved
src/govuk/components/input/template.test.js Show resolved Hide resolved
src/govuk/components/input/template.test.js Show resolved Hide resolved
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 September 1, 2020 18:16 Inactive
@36degrees
Copy link
Contributor

36degrees commented Sep 2, 2020

We want to hold off merging this until we're ready to start on 4.0.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 September 2, 2020 10:54 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 September 2, 2020 10:57 Inactive
@vanitabarrett vanitabarrett added this to the v3.9.0 milestone Sep 2, 2020
@36degrees
Copy link
Contributor

Minor, but can we have at least one example that does not use an input width modifier? This will allow us to verify that the input is unconstrained and 'full-width' by default.

Possibly most of the examples should omit the width modifier? In the same way the National Insurance Number examples are 'full-width' even though in real world usage you'd expect them to be constrained… most of the examples should probably combine the minimum number of 'features' possible? Although we would then want some examples that show that they do combine properly…

@hannalaakso
Copy link
Member

@36degrees I don't know if you saw the feedback from @christopherthomasdesign which the change with the width modifiers was in response to?

But maybe we mainly shouldn't use the width modifiers in the review app as the examples are out of context anyway..? Let me know if you have any thought on that. Definitely happy to include some examples without width modifiers in either case.

@36degrees
Copy link
Contributor

@36degrees I don't know if you saw the feedback from @christopherthomasdesign which the change with the width modifiers was in response to?

Yep, I did – and completely agree for the examples in the Design System.

But maybe we mainly shouldn't use the width modifiers in the review app as the examples are out of context anyway..? Let me know if you have any thought on that. Definitely happy to include some examples without width modifiers in either case.

I think generally we use examples that are focused on only the specific feature, and don't 'enable' anything else – for example, the National Insurance Number examples don't include the spellcheck attribute, the example that enables isPageHeading doesn't also add a class to style it (though I think we might be missing some examples for each label size!)

@hannalaakso
Copy link
Member

hannalaakso commented Sep 3, 2020

That all makes sense, I think I'm just having a slow day!

I've removed the width modifiers from the examples and added a new example that does use one.

The inputs with prefix/suffix actually look pretty strange at full width but I guess that's okay, especially since there isn't an elegant way of setting any kind of max width on them.

Screenshot 2020-09-03 at 18 20 17

@edwardhorsford
Copy link
Contributor

edwardhorsford commented Sep 3, 2020

Do these end up showing in a 2/3 column grid?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1816 September 3, 2020 17:52 Inactive
@hannalaakso
Copy link
Member

hannalaakso commented Sep 3, 2020

Do these end up showing in a 2/3 column grid?

They get constrained by the grid which is also a good point so I think we're okay to leave it as is 👍

simonwhatley and others added 8 commits September 7, 2020 13:20
Doing this makes it possible to include block level elements inside
the prefix/suffix containers.
- Remove browser prefix styles as these will be added by
auto-prefixer.
- Remove redundant margin-top on prefix and display-inline-block
on input
- Make all examples use type=text as both prices and weights can
contain decimal points
Implement design from #1915 (comment)

This ensures that especially on zooming in the input remains visible.

Remove height restriction to ensure that longer prefix/suffix can wrap
instead of overflowing container.

Add an example to demonstrate text wrapping behaviour.
IE8 doesn't support flex so the prefix, suffix and input weren't aligned.

This commit stops the removal of prefix/suffix borders on IE8 to make the
elements look less 'broken'.

There is still a gap between the elements on IE8 which is acceptable
as we only functionally support IE8.
Remove width modifiers from other examples to keep them focused
on the prefix/suffix functionality.

Reorganise examples. We're not going to enforce the ordering of examples
but for now going with the convention of placing the hidden examples at
the end of the yaml.
@hannalaakso
Copy link
Member

Massive thanks for all the amazing work by @simonwhatley and his team on this component 🎉🙏

@hannalaakso hannalaakso merged commit dc5e752 into alphagov:master Sep 7, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Nov 3, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Nov 4, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Nov 6, 2020
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.

Publish prefix / suffix variant for text input
6 participants