Expensify
[$250] Chat - In mWeb, text is centered but in Android text is dropping down
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: V9. 0.64-3 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Issue reported by: Applause Internal Team
Action Performed:
- Open both mWeb and app
- Open a chat
- Enter > # hai
Expected Result:
In mWeb, text is centered but in Android text must not drop down.
Actual Result:
In mWeb, text is centered but in Android text is dropping down.
Workaround:
Unknown
Platforms:
- [x] Android: Standalone
- [x] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [ ] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/d37b1dac-6824-423c-bc2a-bdf2b921ef3e
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021863815952930925048
- Upwork Job ID: 1863815952930925048
- Last Price Increase: 2024-12-10
Issue Owner
Current Issue Owner: @alitoshmatov
Triggered auto assignment to @alexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Proposal
Please re-state the problem that we are trying to solve in this issue.
Headings are not vertically centered on Android.
What is the root cause of that problem?
The line height is enlarged on Android to fix overlapping lines.
However, the text is bottom-aligned in MarkdownLineHeightSpan.
What changes do you think we should make in order to solve the problem?
Fine-tune the fm.ascent, fm.descent, fm.top, fm.bottom properties to vertically center the text. The code below refers to this implementation. It’s not the final version and might need confirmation from the design team.
@Override
public void chooseHeight(CharSequence text, int start, int end, int spanstartv, int lineHeight, Paint.FontMetricsInt fm) {
float leading = mLineHeight - ((-fm.ascent) + fm.descent);
fm.ascent -= (int)Math.ceil(leading / 2.0);
fm.descent += (int)Math.floor(leading / 2.0);
// The top of the first line, and the bottom of the last line, may influence bounds of the
// paragraph, so we match them to the text ascent/descent. It is otherwise desirable to allow
// line boxes to overlap (to allow too large glyphs to be drawn outside them), so we do not
// adjust the top/bottom of interior line-boxes.
if (start == 0) {
fm.top = fm.ascent;
}
if (end == text.length()) {
fm.bottom = fm.descent;
}
}
Screenshots
| Before | After |
|---|---|
What alternative solutions did you explore? (Optional)
N/A
I didn't get to test this one before going OOO. This week, I'll try to log in to confirm the next steps.
@alexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Job added to Upwork: https://www.upwork.com/jobs/~021863815952930925048
Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov (External)
@alitoshmatov - I was able to replicate the experience. Can you please verify if this proposal will fix the issue? Thanks!
@alexpensify @alitoshmatov this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@alexpensify, @alitoshmatov Whoops! This issue is 2 days overdue. Let's get this updated quick!
@QichenZhu Are you suggestion that this should be fixed in react-native-live-markdown
@alexpensify, @alitoshmatov Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@tomekzaw Can you check out this issue where heading in android is pushed down noticeably. And we have this proposal which suggests to update height calculation for android.
I suspect this issue is related to https://github.com/Expensify/react-native-live-markdown/issues/522
What do you think
@alitoshmatov Indeed, this should be fixed on Android with https://github.com/Expensify/react-native-live-markdown/pull/535 telling from the screenshots in https://github.com/Expensify/react-native-live-markdown/pull/535#issuecomment-2525081847.
Looks like we need to center text vertically on iOS (as per this comment https://github.com/Expensify/react-native-live-markdown/pull/535#issuecomment-2525081489) and we're good to go.
Great. I guess we can hold this issue then.
@alexpensify Can you update the issue to hold for https://github.com/Expensify/react-native-live-markdown/pull/535
Heads up, I will be offline until Wednesday, December 18, 2024, and will not actively watch over this GitHub during that period.
If this GitHub requires an urgent update, please ask for help in the #expensify-open-source Slack Room. If the inquiry can wait, I'll review it when I return online.
Weekly Update: https://github.com/Expensify/react-native-live-markdown/pull/535 is still under review
Weekly Update: https://github.com/Expensify/react-native-live-markdown/pull/535 is still under review
Weekly Update: Waiting on https://github.com/Expensify/react-native-live-markdown/pull/535
Weekly Update: https://github.com/Expensify/react-native-live-markdown/pull/535 is still under review
Just FYI, I wasn't actively working on this PR because I was hoping that RN 0.76 upgrade would fix this. Unfortunately this is not the case so I'll need to resume working on this PR once I complete my current tasks.
Oh no, thank you for the update. Keep us posted when you have a better timeline to get to this one. Thanks!