Page MenuHomePhabricator

XHProf application is not optimized for mobile
Closed, ResolvedPublic

Description

The XHProf application looks pretty average on mobile. In particular, the sizing seems off.

Revisions and Commits

Event Timeline

joshuaspence raised the priority of this task from to Needs Triage.
joshuaspence updated the task description. (Show Details)
joshuaspence added projects: XHProf, Design, Mobile.
joshuaspence added a subscriber: joshuaspence.
chad added a subscriber: chad.

I can't imagine someone using this on mobile, is there a need for it o can't think of?

Definitely low priority, but why not?

@joshuaspence my assumption is exactly 0 people use this tool on mobile, And maybe 5 use it on desktop.

epriestley triaged this task as Wishlist priority.Feb 18 2015, 2:50 PM
epriestley added a subscriber: epriestley.

This stretches the bounds of imagination, but I can come up with a use: although we do only a tiny amount of this today, a possible use case is that we may take a different server-side rendering pathway for mobile agents than we do for desktop agents in the future. This does technically happen today, but the amount of code involved is tiny, and is unlikely to ever affect a profile.

However, you could imagine something more drastic in the future, where sections of the page get rendered differently (e.g., omitted on mobile) and the mobile and desktop profiles start to diverge.

More broadly, I think spending a couple of hours here and there to keep lesser-used applications modern is worthwhile. If we have an application which sees so little use that we can't justify bringing it up to spec as the UI and application infrastructure evolve, I think we should consider removing it. But XHProf is a cornerstone infrastructure application and definitely worth keeping modern to me.

Haha, well you know me and killing apps. Always a fan. :-)

OIC, I am confusing XPHAST and XHPROF tasks, I'm a bit jetlagged it seems. Didn't notice two different tasks here.

I almost merged them when they popped up. :P

chad claimed this task.

Seems working fine now on mobile.

I assume you mean the table? That is our mobile display for tables (scrollable).

Oh, on an actual mobile device. We should remove that JS check now, I think it's safe globally.

hmmm maybe this is buildStandardPage thing.

@epriestley is it safe to delete the $is_framed logic in this application? I don't know what it does or how to test, assume a page layout thing.

Removing $is_framed will break the DarkConsole "XHProf" tab, which loads the profile in an <iframe />.

I'm not sure how best to proceed here. What's the difference with buildApplicationPage and buildStandardPage when it comes to device?

I think we can put the frame logic in buildApplicationPage(), and then switch to it. I can tackle that if you aren't sure how to not break stuff.

Typically I'd just switch this over to buildApplicationPage and call it a day, but I don't think that works with frame.

There is a manageable number of buildStandardPage callsites, might be easy to remove if there is only 1-2 missing features.

Definitely desirable, although some of them (releeph, oauth server, arcanist project edit) might be tricky to test.

Should be resolved, probably.