Skip to content
This repository was archived by the owner on Oct 8, 2021. It is now read-only.

Demos: fix style on listview-linkbar demo #7101

Closed
wants to merge 1 commit into from

Conversation

soberstadt
Copy link
Contributor

No description provided.

@coveralls
Copy link

Coverage Status

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

@arschmitz
Copy link
Contributor

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.

@soberstadt
Copy link
Contributor Author

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.

@arschmitz
Copy link
Contributor

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.

@soberstadt
Copy link
Contributor Author

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.

@arschmitz
Copy link
Contributor

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.

@soberstadt
Copy link
Contributor Author

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

@arschmitz
Copy link
Contributor

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

@soberstadt
Copy link
Contributor Author

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

@soberstadt
Copy link
Contributor Author

Good to merge?

@arschmitz
Copy link
Contributor

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 pushed a commit that referenced this pull request Feb 18, 2014
selector updated for 1.4
Fixes gh-7130
Closes gh-7101
@gabrielschulhof
Copy link

This has been applied.

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