Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support retina displays #25

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

aakhmerov
Copy link

  1. add high dpi displays detection
  2. add parameter to skip iPads from style adjustments
  3. add propagation of utility functions to a global window scope object

The minified version of code is not here, please ping me if you'd like me to create one.

2. add parameter to skip iPads from style adjustments
3. add propagation of utility functions to a global window scope object
@nathansmith
Copy link
Owner

Can I ask what the onus is for testing retina in JS?

It seems like this would be better handled in a CSS media query.

(Since any browsers that run on retina are also able to understand media queries.)

@aakhmerov
Copy link
Author

The whole idea of using adaptjs itself is separation of css files, clear and done once. So that desktop and mobile resources get only minor intersections. Now high dpi displays break that concept almost totally as the report high width values to js.

In other words, if I want to have 2 totally different css files for mobile and desktop and retina I would use classical media query based aproach I would have to inline retina styles into desktop css, which is not the best choice,as device itself is mobile.

So i've added retina/high dpi detection to core adapt, so that media queries are still in mobile css files, which is logically correct (except for retina mac pros, but this can be filtered by backend).

@nathansmith
Copy link
Owner

This isn't correct:

"Now high dpi displays break that concept almost totally as the report high width values to js."

Here are screenshots taken at double pixel density (on a retina MacBook Pro) and an external (non-retina) monitor.

RETINA
http://f.cl.ly/items/233f0P2S1R411I443u0r/Screen%20Shot%202013-11-11%20at%2011.54.52%20AM.png

NON-RETINA
http://f.cl.ly/items/2j3v1X07362t082J1h1R/Screen%20Shot%202013-11-11%20at%2011.55.03%20AM.png

In both cases, the browser reports 1171px (just the arbitrary width I had it at) when checking against window.outerWidth.

A "CSS pixel" is not a 1:1 physical screen pixel. The width is reported consistently, whether or not a screen is "retina" density.

Really, unless anyone has to support IE8 (or browsers that don't understand media queries), there's really no reason to use Adapt.js anymore. It's solves a problem that is less and less relevant (same code for IE8 as for modern browsers).

In fact, I'd recommend this approach instead of using Adapt.js nowadays…

http://nicolasgallagher.com/mobile-first-css-sass-and-ie/

@aakhmerov
Copy link
Author

Ok, thanks for the article. But that's approach of enhancing existing css base with additional styles. I'll think again if that's an option in my case, as what I have right now is not the most beautiful thing :)

But, I clearly remember moment of iphone or latest ipod reporting me screen width of 980px, will be able to check that only tomorrow, may be it was my left hand bogous coding at the moment.

On the other hand even if the thing is submiting high value of a screen, I admit that it's a poor excuse to have badly structured css itself, which is not suitable for enhancement.

@nathansmith
Copy link
Owner

If it's reporting the actual pixel width, that's likely because a viewport meta tag wasn't found…

<head>
<meta name="viewport" content="width=device-width, initial-scale=1, minimum-scale=1, maximum-scale=1" />
</head>

Adding that will force the browser to report the width in "CSS pixels" instead of on-screen DPI.

@aakhmerov
Copy link
Author

viewport was in code for sure, now that you've quoted it like that I wonder if it was in head section or not. In any case, will check it all tomorrow, thanks for tips.

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

Successfully merging this pull request may close these issues.

2 participants