Probs to @JoeWalsh for onboarding me to Mobile HTML 👊

@schoenbaechler

06)
Is there a reason why we’re using <i> in the markup here? I think we should remove it to display titles in normal font style not italics. If not possible to remove the <i>, let’s apply this CSS:

The <i> in this case actually comes from the page titles. These games have most their text italicized. I recommend refreshing the page and changing the title in the pcs.c1.Footer.add() call to use a different article than the game Knight Lore one in the example call.

Because of that I advise against changing the font-style to normal in .pcs-footer-readmore-page-container i.

Thanks for the background infos @bearND. Totally makes sense to leave it as is. I removed it from the task’s description.

Change 586419 had a related patch set uploaded (by BearND; owner: BearND):
[mediawiki/services/mobileapps@master] Footer adjustments

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

@schoenbaechler @cmadeo Note that this override some of the changes in T238875. Let me know if we need different design for iOS and Android. (I hope we don't but if it's needed we can do that.)

@schoenbaechler I've created a patch to apply the changes you request in the task description except 02) + 04). I changed the selector from a class selector to an id selector because the element has an id and not a class of these:

#pcs-footer-container-menu,
#pcs-footer-container-readmore {
  margin-bottom: 0;
}

I've made the change for 10) but note that this was recently changed to

html[dir='ltr'] .pcs-footer-menu-item,
body[dir='ltr'] .pcs-footer-menu-item {
  background-position: left 0 center;
  padding-left: calc(25px - (50vw - 50%));
}

html[dir='rtl'] .pcs-footer-menu-item,
body[dir='rtl'] .pcs-footer-menu-item {
  background-position: right 0 center;
  padding-right: calc(25px - (50vw - 50%));
}

Are you sure to make this 4px instead of the calculation?

Here's the before and after in a desktop browser (don't worry about the outer margins -- the apps add those):

Before	After

Thanks, @bearND

In T249134#6033451, @bearND wrote:

@schoenbaechler @cmadeo Note that this override some of the changes in T238875. Let me know if we need different design for iOS and Android. (I hope we don't but if it's needed we can do that.)

Would it be possible to post screenshots from Android and iOS to check it out how it looks in context? It’ll be easier to evaluate for @cmadeo and me. Thx!

@schoenbaechler I've created a patch to apply the changes you request in the task description except 02) + 04). I changed the selector from a class selector to an id selector because the element has an id and not a class of these:

#pcs-footer-container-menu,
#pcs-footer-container-readmore {
  margin-bottom: 0;
}

Makes sense, thx! 👏

I've made the change for 10) but note that this was recently changed to

html[dir='ltr'] .pcs-footer-menu-item,
body[dir='ltr'] .pcs-footer-menu-item {
  background-position: left 0 center;
  padding-left: calc(25px - (50vw - 50%));
}

html[dir='rtl'] .pcs-footer-menu-item,
body[dir='rtl'] .pcs-footer-menu-item {
  background-position: right 0 center;
  padding-right: calc(25px - (50vw - 50%));
}

Are you sure to make this 4px instead of the calculation?

Two things:

01) The separater lines within “About this article" should not go until the right edge of the screen (or left edge in RTL languages):

Implementation	vs	Design
		{F3172053}

I think it has to do with 03 from the task’s description (minus margin) on 100vw:

.pcs-footer-menu-item:after {
border-bottom: 1px solid #eaecf0;
width: calc(100vw - 86px);
}

02) Hmm, yeah it seems like it might be better to keep the calculations, as I don’t know it’s impact on other elements.

The thing I wanted to achieve is that the icons have a 16px margin-left (LTR) / 16px margin-right (RTL) to the edge of the screen on Android.

Also, that the line and the labels are aligned:

Long story short, I think we depend on your expertise here, Bernd. Could you try to make changes to achieve this in the best (progressive) way? Thx!

@schoenbaechler just wanted to double check with you that we want to shorten the line from the right side on (LTR langs) as this is not what we had agreed to in the initial spec: https://phabricator.wikimedia.org/T226094

just wanted to double check with you that we want to shorten the line from the right side on (LTR langs) as this is not what we had agreed to in the initial spec: https://phabricator.wikimedia.org/T226094

@cmadeo and I just chatted about it and agreed on removing the lines completely from the “About this article” section. Would you mind updating it @bearND, please? Thx for the hard work (on details) like this!

@schoenbaechler @cmadeo Ok, I've removed the lines (PS3). Here's a screenshot:

IMO the vertical padding in the "About this Article" area is too big, and in the "Read more" area too small.

The thing I wanted to achieve is that the icons have a 16px margin-left (LTR) / 16px margin-right (RTL) to the edge of the screen on Android.

"Edge of the screen" is relative. It depends on the device dimensions. Did you want me to just move the icons to the left to align where the heading starts?

I'm setting up a way for you to better check this out but it's going to take a bit.

I've set up another CloudVPS instance for trying out UI designs: apps-ui.wmflabs.org.
This one is served through RESTBase, so you can easily get read more articles as well.
You can use this in the browser: https://apps-ui.wmflabs.org/en.wikipedia.org/v1/page/mobile-html/Cat
or in the apps:
(in the Android app developer settings change the RESTBaseURI to https://apps-ui.wmflabs.org/%2$s/v1/).

Here's the same in the Android Nexus 5X emulator:

@schoenbaechler @cmadeo I've added a demo mode to the apps-ui instance. You trigger it by adding the footer=true query parameter.
Feel free to play with the padding values in DevTools on pages like this: http://apps-ui.wmflabs.org/en.wikipedia.org/v1/page/mobile-html/Cat?footer=true and let me know what should be changed.

This can also be combined with the theme parameter:
http://apps-ui.wmflabs.org/en.wikipedia.org/v1/page/mobile-html/Cat?footer=true&theme=dark

Hi @bearND, this looks great thx! Love that we can use parameters to add the footer now! 👏

Minor thing, can we set .pcs-footer-menu-item {padding: 13px 0 17px;} to make both elements 72px high (if there’s one line of description). Currently one is 73px and the other 72px, see screenshots below.

Other than that, good from my end. What do you think @cmadeo ?

@schoenbaechler Ok, just updated this. You'll need to shift-refresh to see the changes.

LGTM, thanks @bearND and @schoenbaechler !

@bearND shift refresh completed – looking good!

Change 586419 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Footer adjustments

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

While running locally and working on T249953 I noticed extra spacing on the footer item icons:

Confirmed with @schoenbaechler that the extra space is unexpected (should align with the margin) and submitting a patch to address this

Change 591046 had a related patch set uploaded (by Joewalsh; owner: Joewalsh):
[mediawiki/services/mobileapps@master] Fix leading spacing of footer menu items

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

Change 591046 abandoned by Joewalsh:
Fix leading spacing of footer menu items

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

Change 591175 had a related patch set uploaded (by Joewalsh; owner: Joewalsh):
[mediawiki/services/mobileapps@master] Fix leading spacing of footer menu items

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

Change 591175 merged by jenkins-bot:
[mediawiki/services/mobileapps@master] Fix leading spacing of footer menu items

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