Page MenuHomePhabricator

Page actions are broken on user pages
Closed, ResolvedPublic1 Estimated Story Points

Description

On user pages, the page actions are broken and show up on top of user links.

pasted_file (563×347 px, 75 KB)

Steps to reproduce -

go to wikipedia mobile -> turn on beta in settings -> visit any user page

Event Timeline

dr0ptp4kt lowered the priority of this task from Unbreak Now! to High.Jun 6 2016, 4:54 PM

As beta is opt-in, reclassifying back to High instead of Unbreak Now. It escaped my attention at first this is beta, not stable.

dr0ptp4kt set the point value for this task to 2.Jun 6 2016, 4:57 PM
jhobs renamed this task from Article actions are broken to Article actions are broken on User pages.Jun 14 2016, 4:45 PM
dr0ptp4kt changed the point value for this task from 2 to 1.Jun 14 2016, 4:49 PM
jhobs renamed this task from Article actions are broken on User pages to Page actions are broken on user pages.Jun 14 2016, 4:49 PM

It should look like this ->

shouldbe.png (798×503 px, 100 KB)

I'm not working on the task, so I'm un-assigning myself.

Change 295775 had a related patch set uploaded (by Bmansurov):
Un-break page actions on User pages

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

@phuedx that patch shouldn't block fixing this, but I think it is a useful refactor.

This is proving more complicated than expected.
If we want a quick short term fix so it doesn't look hideous the following is the most easiest achieved:

Screen Shot 2016-06-27 at 1.50.12 PM.png (517×525 px, 143 KB)

however Baha's batch might be enough given it is beta.

That said, I feel we need a better long term solution. It is my feeling that it would be better to reorganise the HTML so that page actions appears after the user page actions and then try and achieve the floating using tablet specific styles. Relying on absolute positioning as we are currently doing seems problematic and I'm not convinced it will scale well across older devices.

I've +1'd @bmansurov's change and would be happy to merge it.

Despite @Jdlrobson's choice of language, he's correct in saying that the user action styles and DOM layout are complicated. Perhaps we might also take the time them, i.e. put the mobile/tablet/desktop designs, the expected DOM structure and associated styles, and the reasoning behind them all in one place, to alleviate some of this complexity – I take responsibility for this not having been done, by the way.

My reason for +1 (and +2, if necessary), is that, AFAICT, the change removes an obsolete style from the user page styles, which was overriding the more general user action styles.

Relying on absolute positioning as we are currently doing seems problematic and I'm not convinced it will scale well across older devices.

The position property was introduced in the CSS2 recommendation, which was published in 1998. Remember our Grade A, C, and X classifications, and weigh the cost of changing the user actions layout against the cost of supporting what are likely Grade X browsers in this context.

I wasn't quite sure what you meant by "scale" either. With @bmansurov's change checked out, I can arbitrarily add and remove elements from the container without it breaking the layout:

t137054-discussion.gif (369×364 px, 602 KB)

What have I misunderstood?

I think it's me that is misunderstanding.

I merged the patch on the basis that I've not been too involved with the new page actions html/css so I think I'm just confused. Given @phuedx is happy I'm more confident.

@phuedx I would definitely appreciate such a write-up or at least better documentation in code.
My concerns around the CSS are around the fact that I've seen position absolute work strangely in older browsers (but these browsers could now be defunct and not to be worried about) - issues I recall are typically when the parent element is not position relative and the absolute positioning attaches them to elsewhere and problems with the box model. I have no evidence this is a real problem though, but I certainly have run into such issues in the IE6 days (which I guess we don't care about now :))

Change 295775 merged by jenkins-bot:
Un-break page actions on User pages

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

You can test the fix by visiting http://en.m.wikipedia.beta.wmflabs.org/w/index.php?title=User:Baha&mobileaction=beta on a mobile device.

Here is a screenshot too:

Screen Shot 2016-06-29 at 7.00.08 AM.png (1×1 px, 388 KB)