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

Tweak search page styles to match live #318

Merged
merged 9 commits into from Nov 2, 2017
Merged

Tweak search page styles to match live #318

merged 9 commits into from Nov 2, 2017

Conversation

@fofr
Copy link
Contributor

@fofr fofr commented Oct 27, 2017

Minimum changes to get Search looking like live. This doesn't include any work to integrate Search more with finder-frontend (ie shared styles, components, JS and templates come later)

Follow on from #313

  • Include styles for IE8 and below
  • Fix spacing around search bar
  • Fix layout of text when no results
  • Style fallback button when no JS using button component

Master: http://finder-frontend.herokuapp.com/search?q=test
Live: https://www.gov.uk/search?q=test
PR: http://finder-frontend-pr-318.herokuapp.com/search?q=test

Tweaks right border on search

screen shot 2017-10-27 at 17 08 38

No results text

Live:
screen shot 2017-10-27 at 16 56 57

Old:
screen shot 2017-10-27 at 16 41 04

New:
screen shot 2017-10-27 at 16 57 03

Fallback search button

screen shot 2017-10-27 at 16 39 36

Old IE

Old:
screen shot 2017-10-27 at 17 18 48

New:
screen shot 2017-10-27 at 17 19 08

https://trello.com/c/gZYUxr0O/67-help-search-team-move-search-to-finder-frontend-frontend-work

dwhenry and others added 9 commits Oct 16, 2017
* Keep search design the same as frontend
Site search uses a different application stylesheet to avoid scoped
styles affecting finder frontend. These also need their equivalent IE
versions to be available and precompiled.
* Update search styles to match live search spacing
* Styles need adjusting because the empty `<ol>` element from the
breadcrumb, which had some padding, has been removed
The `extra_headers` section was copied from the frontend app, but we
missed the line which includes that section in the template.
Avoid porting button styles by switching to button component:
https://govuk-static.herokuapp.com/component-guide/button

Use same technique as pagination for inserting a component into a
mustache template.
Ideally we’d use the govspeak component (or something similar) to
render this template.

For now, mimic govspeak component’s spacing and list styles.
Avoid extensive refactor for now. Aim is to switch to search component
at a later point – that’s currently hindered by static components not
being interactive in integration tests.
“No results” is when something is searched and nothing is returned.
@fofr fofr changed the title Search frontend tweaks Tweak search page styles to match live Oct 27, 2017
@fofr fofr requested a review from andysellick Oct 30, 2017
@fofr fofr temporarily deployed to finder-frontend-pr-318 Nov 1, 2017 Inactive
Copy link
Contributor

@andysellick andysellick left a comment

Can't comment on the mustache, but otherwise no obvious problems.

border-right: none;
line-height: 48px!important;
line-height: 48px !important;

This comment has been minimized.

@andysellick

andysellick Nov 1, 2017
Contributor

Is there a way of doing this without the !important? I know it's a legacy line, but it seems like a good opportunity to fix it. Unless this'll all change in the next round anyway, not sure what the plan is.

This comment has been minimized.

@fofr

fofr Nov 1, 2017
Author Contributor

I think this will get deleted when we switch to components, so I wasn't too concerned.

@@ -1,27 +1,21 @@
<!DOCTYPE html>
<html lang="en">
<html>

This comment has been minimized.

@nickcolley

nickcolley Nov 1, 2017
Contributor

Should we be removing the lang? Is this added back in when it's injected into the static template?

This comment has been minimized.

@fofr

fofr Nov 1, 2017
Author Contributor

Rails does it by magic.
Note it's still there on: http://finder-frontend-pr-318.herokuapp.com/search?q=test

@dwhenry
dwhenry approved these changes Nov 2, 2017
@fofr fofr merged commit fa918ca into master Nov 2, 2017
1 check passed
1 check passed
continuous-integration/jenkins/branch This commit looks good
Details
@fofr fofr deleted the search-frontend-tweaks branch Nov 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.