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

Fix text input Safari cursor issue #371

Closed
wants to merge 2 commits into from

Conversation

romanr
Copy link

@romanr romanr commented Dec 14, 2016

Fixes #351

Changes proposed in this pull request:

This is fixed by setting line-height: normal in input field css:

input.pt-input.pt-fill {
    line-height: normal;
}

may also be fixed by line-height: 1; or line-height: 100%; however line-height: normal; is considered the best solution.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/blueprint, @romanr! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

@giladgray
Copy link
Contributor

the most important thing here is to ensure that text inputs on Chrome, FF, IE, and Edge are unaffected by this change. have you done some cross-browser testing @romanr?

@giladgray
Copy link
Contributor

@romanr also if you enable CircleCI for your fork then we should get a preview comment!

@adidahiya
Copy link
Contributor

Actually I don't think we get @blueprint-bot comments for forks, as I noted here #349 (comment). Worth looking into

@romanr
Copy link
Author

romanr commented Dec 14, 2016

Yes, It is Tested OK in FF, Chrome, IE.
We added CircleCI to our fork.

@adidahiya
Copy link
Contributor

Can you push an empty commit to this branch to trigger CI?

Copy link
Contributor

@giladgray giladgray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

text is noticeably not centered in input fields on all browsers (safari included) after this change:

input-line-height

large inputs seem unaffected and remain centered after removing line-height.

i think the root cause is that 14px font-size produces 11px-tall letters, whereas 16px font-size (large) produces 12px-tall letters. the odd height means it can't ever be centered on the whole pixel which causes the off-by-a-few issue in the screenshot above.

not sure what to do about this... do we just give up and let it be off-center? i'm sure @llorca will have some words about this.

@llorca
Copy link
Contributor

llorca commented Jan 9, 2017

It's more important for the input text to be correctly centered, rather than the Safari cursor.

I'm still on board to try and fix this Safari weirdness if possible, but this solution won't work because it breaks the vertical alignment of text in inputs.

@giladgray
Copy link
Contributor

Closing as we're not ready to accept this fix: the uncentered text makes us very uncomfortable.

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

Successfully merging this pull request may close these issues.

None yet

6 participants