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

Prevent wrapping of individual date inputs #1257

Closed

Conversation

colinrotherham
Copy link
Contributor

Date inputs should group together as if they're a single field.

This fix keeps them together using white-space: nowrap, and removes the stray right margin from the year (reducing component width to 200px).

Fixes @andysellick's issue on #1250

Issue

@colinrotherham colinrotherham force-pushed the date-input-wrapping branch 2 times, most recently from a8d93c4 to d228116 Compare March 27, 2019 16:46
@colinrotherham
Copy link
Contributor Author

Sorry just spotted :last-of-type is IE9+ so moved margin to the left of siblings instead:

.govuk-date-input__item + .govuk-date-input__item {
  margin-left: govuk-spacing(4);
}

@dashouse dashouse added the awaiting triage Needs triaging by team label Apr 3, 2019
@kellylee-gds kellylee-gds added 🕔 hours A well understood issue which we expect to take less than a day to resolve. Priority: Low and removed awaiting triage Needs triaging by team labels Apr 3, 2019
@colinrotherham
Copy link
Contributor Author

Rebased with master

@hannalaakso
Copy link
Member

Thanks @colinrotherham 🙂 It looks like this will prevent the wrapping of the items even if they don't fit in the container. Here, the date input is inside one of three .govuk-grid-column-one-third and overflows the container on smaller screens.

Screen Shot 2019-04-09 at 16 28 50

I wonder if the fix by GOV.UK Publishing to #1250 might work for us. It doesn't prevent wrapping but it looks a bit neater.

@colinrotherham
Copy link
Contributor Author

Hi @hannalaakso yeah that was intentional. Just like the other fixed-width inputs I thought it shouldn't wrap.

Same if we added the --width-20 modifier.

If it's supposed to wrap then I'm mistaken! Sorry.

I'll amend this pull request to support wrapping with the appropriate vertical spacing.

Thanks

@hannalaakso
Copy link
Member

Thanks @colinrotherham. We can try to get some design clarification to make sure I'm not making wrong assumptions here 😄

@dashouse Would you have any thoughts on how this this should be behave from a design perspective?

@kellylee-gds kellylee-gds requested a review from dashouse April 15, 2019 09:02
@dashouse
Copy link

Thanks for raising this (as always). Sorry to be a blocker, but I don't believe we have a solid decision on how we want the component to behave in this scenario.

The solution to the original issue on GOV.UK will be to "make it fit" in that instance using a few techniques that were discussed in the comment thread. This is because we have some knowns about that layout, for example the minimum width of the filter column.

As global change to this component I think we need to make a few decisions based on unknowns...for example, should we stack inputs, reduce space between inputs, change the input size, consider mobile breakpoint design vs limited width container design, should we be using flexbox for this...etc

For now it's probably best to close this PR, make sure this is properly recorded as an issue and potentially add some options.

@dashouse dashouse closed this Apr 17, 2019
@colinrotherham colinrotherham deleted the date-input-wrapping branch May 16, 2019 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕔 hours A well understood issue which we expect to take less than a day to resolve.
Projects
Development

Successfully merging this pull request may close these issues.

4 participants