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 offset top calculation #2057

Closed
wants to merge 1 commit into from
Closed

Conversation

SailorMax
Copy link

in _showDatepicker() we use native input.offsetHeight
but in _checkOffset() we use jquery's input.outerHeight()

jquery's method returns more accurate height for zoomed out pages. In result _checkOffset()'s offset.top === ( inst.input.offset().top + inputHeight ) never returns true

in _showDatepicker() we use native input.offsetHeight
but in _checkOffset() we use jquery's input.outerHeight()

jquery's method returns more accurate height for zoomed out pages. In result _checkOffset()'s `offset.top === ( inst.input.offset().top + inputHeight )` never returns true
@linux-foundation-easycla
Copy link

CLA Not Signed

@fnagel
Copy link
Member

fnagel commented Mar 9, 2022

Thanks for the report. Does the issue you describe exist when jQuery UI 1.12.1 is used or only with jQuery UI 1.13.0 or newer?

@SailorMax
Copy link
Author

the issue exist when jQuery UI 1.12.1 is used also

Copy link
Member

@mgol mgol left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. We'll need tests for this change, though. Have a look at existing test in https://github.com/jquery/jquery-ui/tree/main/tests/unit/datepicker to see how to write one.

@mgol
Copy link
Member

mgol commented Mar 10, 2022

Also, we'll need you to sign the CLA to proceed.

@SailorMax
Copy link
Author

Also, we'll need you to sign the CLA to proceed.

I'll look later about test. But not ready to sign anything because of 1 tiny patch. You can merge this fix from your name.

@mgol
Copy link
Member

mgol commented Mar 10, 2022

I understand your position but we're obliged to require a CLA signature to accept contributions. If you're not willing to sign the CLA then it's better for someone else to look at it so that ownership is clear. Together with tests this would be much larger than just a single line changed.

Given the above, I'm going to close this, sorry!

@mgol mgol closed this Mar 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants