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

New "all content" finder UI tweaks #3495

Merged
merged 18 commits into from
Oct 10, 2024
Merged

New "all content" finder UI tweaks #3495

merged 18 commits into from
Oct 10, 2024

Conversation

csutter
Copy link
Contributor

@csutter csutter commented Oct 7, 2024

This implements a number of small UI tweaks following a design review:

  • Apply button should not have name attr (this causes the button's text to be added to the query parameters, which we don't need and clutters up the analytics)
  • Improve spacing of page heading by moving overall UI into a container and adjusting margins
  • Fix garbled disclosure marker on filter section details element on Webkit and older Blink browsers (by hiding the ::marker pseudo-element)
  • Fix incorrect "Filter and sort" button underline on Webkit (caused by Webkit not respecting text-decoration-thickness on button elements, and fixed by applying it to an extra inner span instead)
  • Tweak vertical alignment of result count in filter panel summary
  • Improve display of spelling suggestion (by inlining only the bits we need from the existing old finder partial, and tweaking whitespace)
  • Remove standalone page heading and use non-inline label search field with a heading-wrapped label
  • Increase gap between filter panel action buttons
  • Increase padding of filter sections
  • Make text in filter summary more consistently sized (19px)
  • Tweak spacing of tags in filter summary and increase spacing underneath
  • Remove duplicate width container causing display issues at some breakpoints
  • Only use two thirds layout on desktop breakpoint and above (remaining full width on tablet)
  • Tidy and fix broken form markup (tag wasn't closed), and use ERB to generate the form so the view code looks more manageable
  • Use document list component's new equal_item_spacing option to tighten up display of results
  • Remove bottom border on summary component and stop removing the top border on the first document list element instead

Review app: https://finder-frontend-pr-3495.herokuapp.com/search/all

Before

image

After

image

@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 16:30 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 16:46 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 16:49 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 16:54 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 16:59 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 17:09 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 7, 2024 17:15 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 8, 2024 10:19 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 8, 2024 10:33 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 8, 2024 11:14 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 8, 2024 11:16 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 8, 2024 11:31 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 8, 2024 15:07 Inactive
@csutter csutter marked this pull request as ready for review October 8, 2024 16:01
@govuk-ci govuk-ci had a problem deploying to finder-frontend-pr-3495 October 9, 2024 07:11 Failure
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 9, 2024 07:12 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 9, 2024 07:16 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 10, 2024 10:04 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 10, 2024 10:15 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 10, 2024 10:36 Inactive
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 10, 2024 13:08 Inactive
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Nice one, looks good to me. Just a couple of things related to HTML escaping.

app/views/finders/show_all_content_finder.html.erb Outdated Show resolved Hide resolved
app/views/finders/show_all_content_finder.html.erb Outdated Show resolved Hide resolved
This causes the button's text to be added to the query parameters, which
we don't need and clutters up the analytics.
- Move overall UI (except breadcrumbs) into a `app-all-content-finder`
  container
- Add margin to container on tablet+
This needs to be made more resilient for Safari (and some older
browsers) by targeting the marker's (triangle) pseudo-element.
This turned out to be a nasty Webkit bug, which generally seems to be
poor at dealing with text-decoration-thickness.
- Inline spelling suggestion code from old UI partial
- Add wrapper div with appropriate margins
This removes the separate page heading, and uses the variant of the
`search` component that comes with a heading-wrapped label instead to
avoid duplication.
- Reduce margin between heading and "pills"
- Increase gap between "pills"
- Increase bottom padding of component
This still creates a mildly jarring change at 770px but makes things
look nicer on _some_ portrait orientation tablets (iPad is still too
wide to get tablet breakpoint).
There's a `govuk-width-container` within another one, which is
unnecessary and causes display issues at some breakpoints.
Use ERB to generate the form so the source looks more manageable, and
actually close the form element.
This uses the new `equal_item_spacing` option on the document list
component, and returns to using the standard top border on list elements
(and adjusting the filter summary component to remove its bottom border
and tighten its bottom spacing).
@govuk-ci govuk-ci temporarily deployed to finder-frontend-pr-3495 October 10, 2024 13:56 Inactive
Copy link
Member

@kevindew kevindew left a comment

Choose a reason for hiding this comment

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

Thanks

@csutter csutter merged commit e4a63f0 into main Oct 10, 2024
12 checks passed
@csutter csutter deleted the visual-tweaks branch October 10, 2024 14:08
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.

3 participants