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 collapseOnMobile breadcrumbs flag #1754

Merged
merged 3 commits into from
May 28, 2020

Conversation

vanitabarrett
Copy link
Contributor

@vanitabarrett vanitabarrett commented Mar 2, 2020

What

Adds a collapseOnMobile flag to the breadcrumbs component. When set to true, this flag means the breadcrumb component will:

  • Collapse to only showing the first and last item on tablet breakpoint and below
  • Long breadcrumb text will wrap over multiple lines instead of the whole breadcrumb moving to a new line

Note: this change has been implemented on GOVUK and we wish to contribute this change back to the design system.

Why

Screenshot 2020-03-02 at 16 14 10

Screenshot 2020-03-02 at 16 14 15

Screenshot 2020-03-02 at 16 07 54

Screenshot 2020-03-02 at 16 08 02

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1754 March 2, 2020 16:42 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1754 March 4, 2020 16:26 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1754 March 5, 2020 09:45 Inactive
@36degrees 36degrees added this to the Next milestone May 18, 2020
@36degrees
Copy link
Contributor

As discussed on Slack, from talking to the team we’re not 100% sure about the touch target size changes at the minute because:

  • it changes the position of the breadcrumb – because it has a lot more padding, there’s more space between the header and the breadcrumb when using collapseOnMobile, compared to the ‘normal’ breadcrumb or the back link
  • this change wasn't called out when it was put to the working group (which is our fault, sorry!)

We've agreed to just ship the behaviour that hides everything between the first and last breadcrumb, and create an issue to review the touch target size separately (with yourself and the designer that worked on it).

@36degrees
Copy link
Contributor

Raised a separate issue to look at the breadcrumb touch target size: #1817

When this flag is passed, the breadcrumbs component on mobile collapses down to the first and last item only
@vanitabarrett vanitabarrett force-pushed the collapse-breadcrumbs branch 2 times, most recently from 98486ba to 6964eaf Compare May 27, 2020 10:50
@vanitabarrett
Copy link
Contributor Author

Updated PR to remove touch target change but keep:

  • collapsing to first and last item only on mobile
  • Long breadcrumb text will wrap over multiple lines instead of the whole breadcrumb moving to a new line

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1754 May 27, 2020 11:02 Inactive
Copy link
Member

@hannalaakso hannalaakso left a comment

Choose a reason for hiding this comment

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

Thanks for your work on this @vanitabarrett 👍

@36degrees 36degrees merged commit 9a77bee into alphagov:master May 28, 2020
@36degrees 36degrees mentioned this pull request Jun 1, 2020
andymantell added a commit to surevine/govuk-react-jsx that referenced this pull request Jun 2, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 2, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 2, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 2, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 2, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 2, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 23, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Jun 23, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Aug 18, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Aug 18, 2020
alex-ju added a commit to alphagov/govuk_publishing_components that referenced this pull request Aug 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants