Skip to content

Change image placeholder vertical offset when image url is incorrect #688

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

Conversation

jmusial
Copy link
Contributor

@jmusial jmusial commented Jun 2, 2025

Details

Fixes image placeholder vertical offset on web when img url is incorrect.

Related Issues

Closes #687.

Manual Tests

  1. Open Example Web App
  2. Remove some characters from the image url
  3. Incorrect image placeholder displays below the link

Videos

Before

Screen.Recording.2025-06-02.at.16.22.25.mov

After

Screen.Recording.2025-06-02.at.16.21.42.mov

Linked PRs

@jmusial jmusial marked this pull request as ready for review June 2, 2025 14:25
Copy link
Collaborator

@Skalakid Skalakid left a comment

Choose a reason for hiding this comment

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

I saw that on your video image link weirdly jumps when removing a letter from inside of it. I couldn't reproduce it, but can you re-verify please?

Screen.Recording.2025-06-03.at.10.42.15.mov

@jmusial
Copy link
Contributor Author

jmusial commented Jun 4, 2025

@Skalakid I think it's because I have this scroll bar setting on. I have this on main as well. Can fix it but maybe in a separate PR since it's a different issue ? NVM fixed it :)
image

Screen.Recording.2025-06-04.at.13.29.45.mov

@jmusial jmusial requested a review from Skalakid June 4, 2025 11:30
Copy link
Collaborator

@Skalakid Skalakid left a comment

Choose a reason for hiding this comment

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

Left 1 comment. Apart from it, everything looks fine :D

@@ -7,7 +7,7 @@ const spinnerDefaultStyles = {
};
const spinnerContainerDefaultStyles = {
position: 'absolute',
bottom: '0',
bottom: '1px',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe let's add a small comment with an explanation of this change :D

@Skalakid Skalakid merged commit 2d43d98 into Expensify:main Jun 5, 2025
5 checks passed
@os-botify
Copy link
Contributor

os-botify bot commented Jun 5, 2025

🚀 Published to npm in 0.1.286 🎉

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.

Image placeholder covers text when image url is incorrect
2 participants