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

Stop file upload component 'jumping' on focus #2205

Merged
merged 2 commits into from
Apr 27, 2021

Conversation

vanitabarrett
Copy link
Contributor

@vanitabarrett vanitabarrett commented Apr 26, 2021

What

Stop file upload component 'jumping' on focus.

Why

While working on #2194 , we noticed that the file upload component 'jumps' slightly to the left on focus. Example (in Safari, but this affects other browsers too):

Screen.Recording.2021-04-26.at.16.01.58.BEFORE.mov

This is related to some CSS and a comment in the code, which is applied on focus:

// Yank the padding with negative margin to avoid a jump
// when element is focused
margin-right: -$component-padding;
margin-left: -$component-padding;
padding-right: $component-padding;
padding-left: $component-padding;

We can understand why this code was added by observing the effect of removing it:

Screen.Recording.2021-04-26.at.16.06.15.mov

When this CSS is removed, the focus state and the file upload button overlap on the left-hand side.

So... we need to add some padding-left?

The component has bottom and top padding applied by default, but left and right padding is only added on focus (see the code above). What happens if we add padding by default instead of waiting for focus, to prevent anything moving around on focus?

Screenshot 2021-04-26 at 16 07 03

Screenshot 2021-04-26 at 16 07 09

This initially looks good on focus, but we can see that when the focus state is removed the file upload button is out of line with the text above it. This explains why we were initially adding a negative margin.

Setting padding and negative margin by default

Instead of waiting until focus, we can add padding and a negative left margin by default:

Screen.Recording.2021-04-26.at.16.21.11.mov

Cross Browser testing

  • IE11
  • Edge
  • Chrome
  • Firefox
  • Safari
  • Safari (iOS)
  • Chrome (iOS)
  • Chrome (Android)
  • Samsung Internet (Android)
  • Windows High Contrast Mode (Edge)

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2205 April 26, 2021 15:25 Inactive
While working on #2194 , we noticed that the file upload component 'jumps' slightly to the left on focus.

This is because we are applying padding and a negative margin on focus.
This commit instead reworks that CSS to apply the padding and negative margin by default.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2205 April 27, 2021 09:18 Inactive
@vanitabarrett vanitabarrett changed the title WIP Prevent file upload component 'jumping' on focus Apr 27, 2021
@vanitabarrett vanitabarrett changed the title Prevent file upload component 'jumping' on focus Stop file upload component 'jumping' on focus Apr 27, 2021
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2205 April 27, 2021 09:19 Inactive
@vanitabarrett vanitabarrett added this to the v3.12.0 milestone Apr 27, 2021
@vanitabarrett vanitabarrett added file upload 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels Apr 27, 2021
@vanitabarrett vanitabarrett marked this pull request as ready for review April 27, 2021 09:22
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.

Looks good – neat solution and great write up 👍🏻

@vanitabarrett vanitabarrett merged commit c8d2ded into master Apr 27, 2021
@vanitabarrett vanitabarrett deleted the file-upload-focus-jump branch April 27, 2021 10:04
@36degrees 36degrees mentioned this pull request May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) file upload
Projects
Development

Successfully merging this pull request may close these issues.

3 participants