Page MenuHomePhabricator

Prepare branding branch for merging to master
Closed, ResolvedPublic3 Estimated Story Points


To prepare for merging to master and doing a per wiki rollout there are a couple of things that need to be done:

  • Add styles to support cached pages using the old HTML.
  • Add feature flag for enabling/disabling the search icon. When disabled the search icon (magnifying glass) should not show but the search input should (and should take up 100% of the screen on a mobile device). You will need to add specific styles to accommodate the hybrid state. Feature flag should be disabled by default.

A reminder of how to test cached HTML.

  • Pull the latest master branch.
  • `wget http://localhost:8888/w/index.php/Page
  • Move the file retrieved by wget into MobileFrontend repo. Navigate to http://localhost:8888/w/extensions/MobileFrontend/wgetPage.html to view the page and verify it looks and works correctly.
  • Now pull the new branch you want to merge into master. You'll want to rebase this against master e.g. git rebase master or to merge/squash.
  • Open the static page and verify that the page doesn't look broken and behaves correctly. Note that broken in this case means unusable or unacceptable by a designer. It is okay for slight variants given cached pages only live for approx 7 days.

Event Timeline

Restricted Application added a subscriber: Aklapper. ยท View Herald TranscriptFeb 13 2017, 6:24 PM

Change 336352 had a related patch set uploaded (by Jdlrobson):
Adjust new header styles to be compatible with cached HTML

Change 336353 had a related patch set uploaded (by Jdlrobson):
Allow feature flagging of new header

Change 336352 merged by jenkins-bot:
Adjust new header styles to be compatible with cached HTML is the last remaining patch for review that blocks us merging to master.

Notice the padding left of the search button on cached pages when JS and V2 are disabled:

Screen Shot 2017-02-14 at 3.20.55 PM.png (290ร—1 px, 45 KB)

Yup the above issue with the search icon is mentioned in the commit message. Given it only impacts users who are not running JS and is only temporary (1 week) I judged it acceptable. Converting it into the magnifying glass icon is not worth the effort and I think we have to trade off here.

To be clear I was not suggesting to change the button to a magnifying glass, rather to add padding left to the text so that it doesn't touch the button's right border.

padding left can be added but that won't help languages like German where the text is longer... hence why I gave up on trying to make this work. If we see this an issue we should restore the search icon to avoid i18n issues but this will incur an additional icon penalty on users of non-cached pages

โ€ข Jdlrobson set the point value for this task to 3.

This is now ready to go. Have given retroactive estimate based on work that involved.

Change 336353 merged by jenkins-bot:
Allow feature flagging of new header

โ€ข Jdlrobson updated the task description. (Show Details)

Given is passing I'm calling this ready to go.