Demos: fix style on listview-linkbar demo #7101

Closed
wants to merge 1 commit into
from

Projects

None yet

4 participants

Contributor

No description provided.

Coverage Status

Coverage remained the same when pulling c127181 on soberstadt:master into b7f0411 on jquery:master.

Owner

Thank you for noticing this issue and submitting a PR, however you are also changing the font size which was correct. While I admit the font size may be small on desktop, anything larger will not fit on a mobile screen. We should probably use a media query here to adjust based on screen size. However we don't want to change to 14px here. If you would like to either change this back to 8 or try adding a media query the selector change looks good.

Contributor

Well, I would disagree with it the 14px font not fitting. Here are some screenshots:
Testing using Chrome emulating an iPhone 5

With 8px

screen shot 2014-02-17 at 9 34 44 pm

With 14px font

screen shot 2014-02-17 at 9 34 29 pm

I would be up for changing it to 12px or even writing a media query, but I don't think 8 is the right value to have right now.

Owner

Sorry to be a pain here but it looks like from you screen shots you are testing with a screen size double that of an iPhone ( you have viewport of 640 X something ). Please see attached screen shots with iPhone.
an iPhone 5 screen is only 320 × 444 iPhone 4 is only 320 x 416 ( even smaller then screen shots )
With 8px:
photo 1
With 14px:
photo 2
As I said before if you would like to change this back to 8px, or add media queries I would be happy to land this. However i cant do so with 14 or 12px as they wont work well on mobile screens.

Contributor

You're right. You're not being difficult. I think good demos are important. You're right about the text size, I should have tested it in the emulator first.

This demo as a whole is pretty messed up on iOS 7 safari. Once you scroll the sorter is floating in the middle of the page. Perhaps that means I need to do some more work on it.

Owner

I actually noticed this while looking at your PR. There was a library change that was preventing the height adjustment I'm fixing this already so don't worry about that.

One thing to note about use of this on iOS though. iOS prevents javascript execution while scrolling. So ( once i push the fix ) the height adjustment will only take place once you stop scrolling and not during scrolling like in chrome or similar on desktop.

Contributor

Ok, I'll set the font back to 8px.

Owner

@soberstadt Feel free to implement a media query if you feeling ambitious. :)

Contributor

After you fix the viewport height issues, i might clean it up some.

Contributor

Good to merge?

Owner

Yes it looks good. I'm going to add this commit to some others to fix other issues with this demo. Ill push all fixes for the demo soon.

@arschmitz arschmitz added a commit that referenced this pull request Feb 18, 2014
@soberstadt @arschmitz soberstadt + arschmitz Listview: fix style on listview-linkbar demo
selector updated for 1.4
Fixes gh-7130
Closes gh-7101
11c0ed4
Contributor

This has been applied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment