Skip to content
This repository has been archived by the owner on Apr 17, 2018. It is now read-only.

mobile-style: improve mobile display #597

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

@brendanlong
Copy link
Contributor

It's interesting that we both did this at the same time, see PR #599.

The reasons I think my approach is better are:

  • It doesn't require a completely different phone theme (the only media query I use is to remove the tagline from the discussion page)
  • It scales fluidly in the range it supports (yours would have huge text on a screen that's 750px wide).

Also, I think yours has a bug where if you scale a desktop window to less than 750px wide, it makes the text huge. I assume you could fix this by making the media queries check if the device is a phone or something.

Mine doesn't scale as far as yours, but that's only because I didn't remove the sidebar.

Separate the narrow-viewport changes, which should apply to all mobile devices, from the make-things-big changes, which should only apply to phones.  Triggers this difference off of the device-width: phones have a low-device width while tablets have a higher (though still relatively low) one
@jeffkaufman
Copy link
Author

if you scale a desktop window to less than 750px wide, it makes the text huge.

This was also affecting tablets. Fixed with b9a1bde, which gates the font size increases on a max-device-width of 550px.

Screenshots from Chrome's emulated ipad:

before: http://www.jefftk.com/lw-testing/discussion-ipad-old.png
after: http://www.jefftk.com/lw-testing/discussion-ipad-new.png

@jack-trikeapps
Copy link
Contributor

jack-trikeapps commented Jan 4, 2017

Hi Jeff, Brendan,

I'm trying to decide which of your two PRs I should merge. I'm currently leaning towards this one, for the following reasons:

  • Similarity to eaforum, in case cross-porting changes becomes necessary
  • Lack of JS
  • Seems to work better in Firefox's responsive testing mode, where Brendan's doesn't.

Brendan's comments about smooth scaling sound useful, is there a way to get the best of both worlds?

@brendanlong
Copy link
Contributor

  • Mine works perfectly in Firefox's responsive testing mode. Do you have this one backwards?
  • The JavaScript isn't a problem: It only matters on mobile browsers, which don't let you turn JavaScript off, and if you somehow managed to do it, the only problem would be the initial scale being slightly worse on phones (it would show the page at "actual size" instead of zoomed out to show the entire page). Also, we will probably want JavaScript anyway if we hide the sidebar, since we'd want a way to show it again.

I'd also add:

  • The CSS in mine is a lot easier to understand and test, since there's one theme with small changes on smaller screens, instead of effectively two completely different themes.
  • I'm not sure where the sidebar went in Jeff's version. How do we log in?

@jeffkaufman
Copy link
Author

@brendanlong "I'm not sure where the sidebar went in Jeff's version. How do we log in?"

It's at the bottom. For example, see (from above):

http://www.jefftk.com/lw-testing/discussion-old-bottom.png
http://www.jefftk.com/lw-testing/discussion-new-bottom.png

@jeffkaufman
Copy link
Author

@jack-trikeapps Thanks for looking it over!

Brendan's comments about smooth scaling sound useful, is there a way to get the best of both worlds?

With b9a1bde there's no longer a problem of huge text on desktops and tablets. Is there something else you like out of the more responsive version?

@brendanlong
Copy link
Contributor

Ah cool. Moving the sidebar is a good idea.

@brendanlong
Copy link
Contributor

brendanlong commented Jan 7, 2017

@jack-trikeapps I'm worried that we're making perfect the enemy of good here. Less Wrong is currently unusable on phones, so I'd like to propose:

  • If you're leaning towards Jeff's solution, merge it now.
  • I can rebase my solution on top of that commit so we can have a conversation about which approach is better in the long term. This will also make the comparison easier since it will be a two-way rather than three-way choice.

@AndrewPratley
Copy link
Contributor

AndrewPratley commented Mar 23, 2017

Hi @jeffkaufman

There were a couple of issues which would be good if you could fix before we deploy (see attached screenshots):

  • We just changed the homepage. Oliver H and Matthew G are going to fix the css on it shortly (you might want to wait until they have done this). Your changes end the page prematurely where the sidebar was. See here: sidebar
  • The Privacy and Terms gets bumped down slightly. See here: privacy
  • Show By is showing two arrows. See here: arrows

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants