Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.92k forks source link

Chat - In mWeb, text is centered but in Android text is dropping down #52827

Open IuliiaHerets opened 5 days ago

IuliiaHerets commented 5 days ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com 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:

  1. Open both mWeb and app
  2. Open a chat
  3. 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:

Screenshots/Videos

https://github.com/user-attachments/assets/d37b1dac-6824-423c-bc2a-bdf2b921ef3e

View all open jobs on GitHub

melvin-bot[bot] commented 5 days ago

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.

QichenZhu commented 3 days ago

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.

Screenshot 2024-11-23 at 1 27 58 AM

However, the text is bottom-aligned in MarkdownLineHeightSpan.

Screenshot 2024-11-23 at 1 40 05 AM

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
Before After

What alternative solutions did you explore? (Optional)

N/A

alexpensify commented 5 hours ago

I didn't get to test this one before going OOO. This week, I'll try to log in to confirm the next steps.