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

[lexical-playground] Bug Fix: Replace empty <img> tags with invisible <span> elements to prevent Safari issue #6836

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

snikhil24
Copy link

@snikhil24 snikhil24 commented Nov 15, 2024

Description

Describe the changes in this pull request

  • Current Behavior: Equations display with question marks around them in Safari due to empty tags used for spacing around the KaTeX equation
  • Changes: This PR replaces the empty tags with invisible elements for spacing, which resolves the issue in Safari while maintaining functionality across other browsers.

Closes #6824

Test plan

Before

Insert relevant screenshots/recordings/automated-tests

Screenshot 2024-11-14 at 11 54 18 PM

After

Insert relevant screenshots/recordings/automated-tests

image

Copy link

vercel bot commented Nov 15, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 4:56am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 4:56am

@facebook-github-bot
Copy link
Contributor

Hi @snikhil24!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

Copy link

size-limit report 📦

Path Size
lexical - cjs 30.85 KB (0%)
lexical - esm 30.73 KB (0%)
@lexical/rich-text - cjs 39.58 KB (0%)
@lexical/rich-text - esm 32.67 KB (0%)
@lexical/plain-text - cjs 38.22 KB (0%)
@lexical/plain-text - esm 29.93 KB (0%)
@lexical/react - cjs 41.35 KB (0%)
@lexical/react - esm 34.03 KB (0%)

Copy link
Collaborator

@etrepum etrepum left a comment

Choose a reason for hiding this comment

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

Did you test this on Android? It sounds like this workaround is specifically for an Android related purpose.

@etrepum
Copy link
Collaborator

etrepum commented Nov 15, 2024

See also #4040 #1968

@snikhil24
Copy link
Author

I've tested the changes on Android using a Pixel device in two ways:

  • On an Actual Mobile Device: I accessed the application through the browser on my Pixel phone and confirmed that the equations rendered correctly.

  • Via Browser Simulation: I also used the 'Toggle Device Toolbar' feature in Chrome DevTools to simulate a Pixel device and verified the behavior.

Both tests showed the issue is resolved, and the equations render as expected with the invisible elements in place of the empty tags.

Since I’m new to open source contribution, I’d appreciate your input on whether this is the correct approach for testing Android behavior.

@etrepum
Copy link
Collaborator

etrepum commented Nov 15, 2024

In this particular case the reason the images are there are for input reasons rather than rendering reasons. I think your testing approach is correct, but it sounds like you were testing the wrong thing (rendering instead of input). Probably the best thing to do here would be to set CSS and/or an src on the img tags so that they don't show placeholders on Safari, which is less likely to cause an input regression on Android.

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.

Bug: empty images around equations render as question marks on Safari
3 participants