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(overlay-mixin): remove scroll lock on ios and mac #2478

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BogdanDiaconu97
Copy link

@BogdanDiaconu97 BogdanDiaconu97 commented Feb 17, 2025

What I did

  1. Removed css classes because I didn't find bugs related to inputs (maybe they were fixed in earlier versions)
  2. Removed the test about this edge case
  3. Removed unused imports

Copy link

changeset-bot bot commented Feb 17, 2025

🦋 Changeset detected

Latest commit: 8c185ec

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@lion/ui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@CLAassistant
Copy link

CLAassistant commented Feb 17, 2025

CLA assistant check
All committers have signed the CLA.

@tlouisse
Copy link
Member

tlouisse commented Feb 24, 2025

Removed css classes because I didn't find bugs related to inputs (maybe they were fixed in earlier versions)
Removed the test about this edge case
Removed unused imports

Hey, it would be really great if we could remove this abstraction. For that, we need to be 100% sure that the problem does not occur in all Safari versions we support (we support latest three versions (that is 18, 17 and 16) so that older iOS versions are covered).
If it's still an issue on Safari, the fix will still be very valuable

About ~1.5 year ago @DannyMoerkerke worked on this and back then, it was still needed. (let's say we needed to support Safari 14/15 then). I hope he can provide us with the exact reproduction setup.

So, how can we confirm that it is not needed anymore?

  • reproduce the issue on an older Safari version. Let's say v14.
  • confirm that this issue has been resolved in Safari v16.

@BogdanDiaconu97 could you take a look at the above?

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.

3 participants