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

Dynamically position text within input prefixes and suffixes #4157

Merged
merged 1 commit into from
Jan 2, 2024

Conversation

owenatgov
Copy link
Contributor

What/Why

Use flexbox to vertically and horizontally position the text within the input prefix and suffix elements and remove the custom line height being used to nudge the text on smaller screens.

Fixes #3900. Details in the issue.

@github-actions
Copy link

github-actions bot commented Aug 31, 2023

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 113.79 KiB
dist/govuk-frontend-development.min.js 38.58 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 78.74 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 73.99 KiB
packages/govuk-frontend/dist/govuk/all.mjs 3.86 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 113.78 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 38.57 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.38 KiB

Modules

File Size
all.mjs 70.32 KiB
components/accordion/accordion.mjs 21.67 KiB
components/button/button.mjs 4.7 KiB
components/character-count/character-count.mjs 21.24 KiB
components/checkboxes/checkboxes.mjs 5.83 KiB
components/error-summary/error-summary.mjs 6.57 KiB
components/exit-this-page/exit-this-page.mjs 16.08 KiB
components/header/header.mjs 4.46 KiB
components/notification-banner/notification-banner.mjs 4.93 KiB
components/radios/radios.mjs 4.83 KiB
components/skip-link/skip-link.mjs 4.39 KiB
components/tabs/tabs.mjs 10.16 KiB

View stats and visualisations on the review app


Action run for db78328

@owenatgov owenatgov requested a review from a team August 31, 2023 14:41
display: inline-block;
// Use flexbox to align text within the prefix and suffix
display: flex;
align-items: center;
Copy link
Contributor

@colinrotherham colinrotherham Sep 1, 2023

Choose a reason for hiding this comment

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

Works great 👍

Will it be an issue that the input still has line-height: 1.25?

We've got 2x vertical alignment methods to keep in sync now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good spot. From a quick play around this doesn't seem to make a difference for me. I'll do some digging and try to understand what's going on...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah silly me. This is coming from govuk-font. I'm personally not worried about this as I'm viewing the prefixes and suffixes as purely visual aids rather than necessarily elements of the typography. I'm also not totally sure it makes a difference for the inputs since they're already driven by padding and flexbox.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it's not a worry that's fine. Wondered if you wanted the same approach on both?

@owenatgov owenatgov marked this pull request as ready for review December 15, 2023 16:21
@colinrotherham
Copy link
Contributor

@owenatgov Review wise, are we accepting there's still a very slight misalignment?

During testing of the govuk-frontend components against the new typography scale, we noticed that prefixes and suffixes were both now slightly vertically misaligned on small screens.

It's better than what the issue describes, but close enough to call it done?

Safari macOS iOS Simulator

@owenatgov owenatgov force-pushed the input-prefix-suffix-dynamic-positioning branch from 3172db5 to bf5bccb Compare December 18, 2023 16:04
Copy link

github-actions bot commented Dec 18, 2023

Stylesheets changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
index 7036eb68b..403e85fb3 100644
--- a/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
+++ b/packages/govuk-frontend/dist/govuk/govuk-frontend.min.css
@@ -2885,7 +2885,9 @@ screen and (forced-colors:active) {
     font-size: 1rem;
     line-height: 1.25;
     box-sizing: border-box;
-    display: inline-block;
+    display: flex;
+    align-items: center;
+    justify-content: center;
     min-width: 2.5rem;
     height: 2.5rem;
     padding: 5px;
@@ -2923,14 +2925,6 @@ screen and (forced-colors:active) {
     }
 }
 
-@media (max-width:40.0525em) {
-
-    .govuk-input__prefix,
-    .govuk-input__suffix {
-        line-height: 1.6
-    }
-}
-
 @media (max-width:19.99em) {
 
     .govuk-input__prefix,

Action run for db78328

Copy link

github-actions bot commented Dec 18, 2023

Other changes to npm package

diff --git a/packages/govuk-frontend/dist/govuk/components/input/_index.scss b/packages/govuk-frontend/dist/govuk/components/input/_index.scss
index 8e276be74..b236edf84 100644
--- a/packages/govuk-frontend/dist/govuk/components/input/_index.scss
+++ b/packages/govuk-frontend/dist/govuk/components/input/_index.scss
@@ -119,26 +119,21 @@
   .govuk-input__prefix,
   .govuk-input__suffix {
     @include govuk-font($size: 19);
-
     box-sizing: border-box;
-    display: inline-block;
+    // Use flexbox to align text within the prefix and suffix
+    display: flex;
+    align-items: center;
+    justify-content: center;
     min-width: govuk-px-to-rem(40px);
     height: govuk-px-to-rem(40px);
     padding: govuk-spacing(1);
     border: $govuk-border-width-form-element solid $govuk-input-border-colour;
     background-color: govuk-colour("light-grey");
-
     text-align: center;
-    @include govuk-media-query($until: tablet) {
-      line-height: 1.6;
-    }
     white-space: nowrap;
-
     // Emphasise non-editable status of prefixes and suffixes
     cursor: default;
-
     flex: 0 0 auto;
-
     // Split prefix/suffix onto separate lines on narrow screens
     @include govuk-media-query($until: mobile) {
       display: block;

Action run for db78328

Copy link
Member

@querkmachine querkmachine left a comment

Choose a reason for hiding this comment

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

Looks rockin' to me 🤘

We use flex on the prefix and suffix elements themselves to vertically and horizontally centre the text inside these elements. This is opposed to using a custom line height to move text closer to the centre on small screens.
@owenatgov owenatgov force-pushed the input-prefix-suffix-dynamic-positioning branch from bf5bccb to db78328 Compare January 2, 2024 13:16
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4157 January 2, 2024 13:17 Inactive
@owenatgov
Copy link
Contributor Author

@colinrotherham So it's on record, I think that this alignment is ok. I'm open to being disagreed with if the feelings are strong.

@colinrotherham
Copy link
Contributor

@owenatgov Fine by me, was only checking against the "Why" in the issue to see if it was complete

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.

Review line height of prefix and suffixes for text inputs
4 participants