Skip to content

Fix display issues with pasted or uploaded images #3117

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

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

Conversation

askeew
Copy link

@askeew askeew commented Jul 3, 2025

Summary

  • Fix aspect ratio, use fit
  • Remove fixed height on image frame to align close button on image
  • Use overlay instead of ZStack to reduce complexity
  • Add background to close button to get better contrast in light mode
  • Change close-image to be a button for better accessibility

Checklist

  • I have read (or I am familiar with) the Contribution Guidelines
  • I have tested the changes in this PR
  • I have opened or referred to an existing github issue related to this change.
  • My PR is either small, or I have split it into smaller logical commits that are easier to review
  • I have added the signoff line to all my commits. See Signing off your work
  • I have added appropriate changelog entries for the changes in this PR. See Adding changelog entries
    • I do not need to add a changelog entry. Reason: [Please provide a reason]
  • I have added appropriate Closes: or Fixes: tags in the commit messages wherever applicable, or made sure those are not needed. See Submitting patches

Test report

Device: iPhone 15, iPhone 13

iOS: 18.5, 17.6.1

Damus: 1.12 (682) 4cf8097, 1.15 (1) ce19e12d

Steps: uploaded and pasted image and videos, tried both single image/video and multiple

Results:

  • PASS

before multiple:
before-multiple

after multiple:
multi

before single:
before-single

after single:
sing

@danieldaquino
Copy link
Collaborator

Thank you @askeew, I will take a look and review it soon — likely on Wednesday.

Copy link
Collaborator

@danieldaquino danieldaquino left a comment

Choose a reason for hiding this comment

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

Thank you @askeew. I added one inline comment.

One other comment is about the commit message format. Please ensure your commit messages follow our contribution guidelines.

Please let me know if you have any questions.

.contextMenu { contextMenuContent(for: media[index]) }
} else {
KFAnimatedImage(media[index].uploadedURL)
.imageContext(.note, disable_animation: false)
.configure { view in
view.framePreloadCount = 3
}
.aspectRatio(contentMode: .fit)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably use .fill instead. .fit causes the image to look dissociated with the close button when the image has a more "eccentric" aspect ratio. Examples:

IMG_3591
IMG_3590
IMG_3592

@askeew askeew force-pushed the 2913-image-display-bug branch from 10e3f3c to bc967b4 Compare August 4, 2025 15:15
- Fix aspect ratio, use fit
- Remove fixed height on image frame to align close button on image
- Use overlay instead of ZStack to reduce complexity
- Add background to close button to get better contrast in light mode
- Change close-image to be a button for better accessibility

Changelog-Fixed: Fix aspect ratio on pasted or uploaded images
Signed-off-by: Askeew <[email protected]>
Closes: damus-io#2913
@askeew askeew force-pushed the 2913-image-display-bug branch from 85a319c to 469980a Compare August 5, 2025 09:39
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.

2 participants