-
Notifications
You must be signed in to change notification settings - Fork 156
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 oversize padding on captioned images/videos #3732
Fix oversize padding on captioned images/videos #3732
Conversation
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3732 +/- ##
===========================================
- Coverage 83.06% 83.04% -0.02%
===========================================
Files 1754 1755 +1
Lines 44022 44032 +10
Branches 5144 5148 +4
===========================================
+ Hits 36565 36566 +1
- Misses 5635 5638 +3
- Partials 1822 1828 +6 ☔ View full report in Codecov by Sentry. |
...es.messages.impl.timeline.components.reactionsummary_ReactionSummaryViewContent_Day_0_en.png
Outdated
Show resolved
Hide resolved
Use consistent padding with the InReplyToView for the media, and consistent caption padding with other textual messages. Signed-off-by: Joe Groocock <[email protected]>
cc71748
to
012e15e
Compare
Nice, thanks Jorge! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. We can always iterate on it later, but it does look more like the figma designs.
It's a shame the screenshots are kind of broken, but that's a bug in Paparazzi as you mentioned.
Content
Use consistent padding with the InReplyToView for the media, and consistent caption padding with other textual messages.
Note: The screenshots look kinda broken, but it looks like they were broken before my changes. This renders fine in-app and I've been using this patch for several weeks already.
Motivation and context
It bothered me that the padding around captioned images vs reply blocks was different. Judging by element-hq/element-x-ios#3436 it looks like the behaviour in this PR is more aligned with what iOS does too.
This change does also suffer from the issue outlined in element-hq/element-x-ios#3436 too although I'm not sure that's a new problem.
Screenshots / GIFs
Tests
Tested devices
Checklist