Page MenuHomePhabricator

Replace the search bar from the header on small screens with the search icon
Closed, ResolvedPublic2 Story Points

Description

Currently the search bar looks like this:

It needs to be changed to look like this:

Signed in

New notification

New notification - viewed

Acceptance Criteria

  • Remove it and add the search icon next to the notifications icon as seen below:
  • Clicking on the icon should trigger the search overlay (similar to how clicking on the search bar triggers the search overlay)
  • The change doesn't affect the tablet mode, only small screens are affected. JR suggested: Input no longer shows in mobile or tablet mode to simplify implementation of T152459
  • The change needs to be done in the feature branch called branding.
  • make sure tapping the icon pops up the keyboard and textfield is in :focus. typing search term should be not take 2 taps.

Specifications

https://zpl.io/2ckgFh
or see all mocks tagged by this phab card
https://app.zeplin.io/project.html#pid=57a120dbaa97eeab3c8805ae&dashboard&tags=branding

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
ovasileva updated the task description. (Show Details)Dec 19 2016, 5:29 PM

Change 330354 had a related patch set uploaded (by Jdlrobson):
New chrome header with branding

https://gerrit.wikimedia.org/r/330354

Change 330357 had a related patch set uploaded (by Jdlrobson):
New chrome header with branding

https://gerrit.wikimedia.org/r/330357

Change 330357 abandoned by Jdlrobson:
New chrome header with branding

Reason:
Should be on branding branch

https://gerrit.wikimedia.org/r/330357

Change 330359 had a related patch set uploaded (by Jdlrobson):
Brand beta differently

https://gerrit.wikimedia.org/r/330359

Not a 2 pointer... at least a 3 but more likely a 5. There are now 2 patches in code review.

The above changes also impact tablet experience (since it's a feature branch I figured a mobile first from scratch approach was the correct on here).
Note the tablet experience is going to be more tricky and I haven't tackled that yet... add 3-5 points for that alone.
and it seems we have neglected in the mocks what should happen for non-JS users....

Change 330378 had a related patch set uploaded (by Jdlrobson):
Brand beta differently

https://gerrit.wikimedia.org/r/330378

Change 330378 abandoned by Jdlrobson:
Brand beta differently

Reason:
wrong branch

https://gerrit.wikimedia.org/r/330378

Change 330381 had a related patch set uploaded (by Jdlrobson):
Search input shown on tablet resolutions

https://gerrit.wikimedia.org/r/330381

The above changes also impact tablet experience (since it's a feature branch I figured a mobile first from scratch approach was the correct on here).
Note the tablet experience is going to be more tricky and I haven't tackled that yet... add 3-5 points for that alone.
and it seems we have neglected in the mocks what should happen for non-JS users....

The tablet work is tracked under https://phabricator.wikimedia.org/T152459 - should we increase the estimate on that?

I've done more than the task asks, as I got a little confused by the attached mocks and scope of this work. I've attached patches to other tasks.

The change doesn't affect the tablet mode, only small screens are affected.

This acceptance criteria is what is making this tricky for me. It makes more sense to gut the entire header if we are doing this on a feature branch and restore search in tablet mode later. Can I suggest we move this to the tablet card? https://gerrit.wikimedia.org/r/#/c/330354/8 captures all the work in this card but breaks tablet and no-js experience - which is fine for a feature branch.

Jdlrobson updated the task description. (Show Details)Jan 5 2017, 4:10 AM

@Jdlrobson - it makes sense. We knew these were interconnected prior to splitting them up. I guess it would make sense to move the patch to the tablet card and move the card into this sprint? All related tasks are listed as subtasks under T148514: [EPIC] Improve site branding on Mobile website

(It's okay just to work on this patch in this sprint provided it is acceptable for the branch to finish in a non-deployable state.)

To be clear https://gerrit.wikimedia.org/r/#/c/330354/ is the only patch I am seeking review for now.

Change 330354 merged by jenkins-bot:
New chrome header with branding

https://gerrit.wikimedia.org/r/330354

can i see this change? on staging or something?

@Jdlrobson - could you deploy to reading-web-staging?

IT's deployed, but remember this is a WIP and all it does is add the search icon.
http://reading-web-staging.wmflabs.org/w/index.php?title=Main_Page&mobileaction=toggle_view_mobile

Don't worry we will have a review after all the other tasks are done and we are ready to merge to master :)

@Jdlrobson :) okay, i guess i get worried when i see patch merged. old habit. thanks!

Nirzar updated the task description. (Show Details)Jan 18 2017, 6:59 PM
Nirzar updated the task description. (Show Details)
Nirzar added a comment.EditedJan 18 2017, 10:07 PM

zeplin failed to add padding to SVGs

manually exported SVGs

feedback based on reading web staging

  1. the inset shadow for .header-container is missing.
box-shadow: inset 0px -1px 3px rgba(0,0,0,0.08)
Very imp: the border-bottom transparent shouldn't be there but i don't know why it's there so let's talk about removing it *
  1. might be related to border bottom but when i trigger search, the whole thing moves up by 1px which is very apparent and we need to fix it. it looks unstable
  1. the search icon is wrong size.

Change 334226 had a related patch set uploaded (by Jdlrobson):
Add box shadow to headers

https://gerrit.wikimedia.org/r/334226

Change 334227 had a related patch set uploaded (by Jdlrobson):
Add box shadow to headers

https://gerrit.wikimedia.org/r/334227

Change 334226 abandoned by Jdlrobson:
Add box shadow to headers

Reason:
wrong branch again innit

https://gerrit.wikimedia.org/r/334226

https://gerrit.wikimedia.org/r/334227 fixes the border problem.
@Nirzar will upload an SVG to allow us to fix the other issue.
All recent changes are on reading web staging.

phuedx added a subscriber: phuedx.

@Jdlrobson: Is this ready for review or should we wait until @Nirzar's review is Done?

The above patch has been +2'ed. I see some SVG's up above in comments. Are we waiting for new ones?

Change 334227 merged by jenkins-bot:
Add box shadow to headers

https://gerrit.wikimedia.org/r/334227

Yup waiting on @Nirzar for a new SVG now.

Jdlrobson removed Jdlrobson as the assignee of this task.Jan 26 2017, 7:31 PM

Change 334425 had a related patch set uploaded (by Jdlrobson):
Update notification and search icons

https://gerrit.wikimedia.org/r/334425

Change 334427 had a related patch set uploaded (by Jdlrobson):
Update notification and search icons

https://gerrit.wikimedia.org/r/334427

Change 334425 abandoned by Jdlrobson:
Update notification and search icons

Reason:
Nope should be branding. Doh.
https://gerrit.wikimedia.org/r/#/c/334427/

https://gerrit.wikimedia.org/r/334425

@Jdlrobson

.header .branding-box h1 img {
position:relative;
top:-2px;
}

aligning the logo vertically center to the header

Change 334443 had a related patch set uploaded (by Jdlrobson):
Do not downscale the chrome header logo

https://gerrit.wikimedia.org/r/334443

With https://gerrit.wikimedia.org/r/334443 applied I see the logo has become bigger that the one displayed in the mock-up. Also there right border is touching the search input.

Change 334427 merged by jenkins-bot:
Update notification and search icons

https://gerrit.wikimedia.org/r/334427

Jdlrobson added a comment.EditedJan 27 2017, 1:19 AM

@bmansurov which logo are you using? You'll need to specify width and heights in the config. See configuration changes in https://phabricator.wikimedia.org/T156423

$wgMFCustomLogos = [
 'copyright' => '.../brand.svg',
 'copyright-width' => 118,
  'copyright-height' => 18
];

Change 334443 merged by jenkins-bot:
Do not downscale the chrome header logo/align it vertically

https://gerrit.wikimedia.org/r/334443

phuedx reassigned this task from Jdlrobson to Nirzar.Jan 30 2017, 10:15 AM
Nirzar added a comment.EditedJan 30 2017, 6:27 PM

The wikipedia logo is still blurry on staging. needs more work

@Nirzar does this only happen on retina displays by any chance..?

Nirzar added a comment.EditedJan 30 2017, 10:36 PM

most probably, same reason we have @2x versions on portal.

but SVGs work the best. device density independent. or background size works good too.

Change 335165 had a related patch set uploaded (by Jdlrobson):
SVG logo should not blur

https://gerrit.wikimedia.org/r/335165

@Nirzar this patch is enabled on the reading web staging instance for you to verify this fixes the problem

@Nirzar this patch is enabled on the reading web staging instance for you to verify this fixes the problem

I'd like to take a look but I don't know where that staging instance is. Can you point me to it? Thanks!

Change 335165 abandoned by Jdlrobson:
SVG logo should not blur

Reason:
SVG was the issue

https://gerrit.wikimedia.org/r/335165

We fixed this by using a new SVG. The downside of using an SVG is a broken image will be rendered in Android 2, but given this is about 1% of traffic this might be okay for the time being (since it's just the logo).

Signed off from Design! logo looks sleek ;)