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
2.99k stars 2.5k forks source link

[No QA][VIP-Travel] Trip room summary #41659

Open rushatgabhane opened 1 week ago

rushatgabhane commented 1 week ago

Details

Fixed Issues

$ https://github.com/Expensify/App/issues/37825 PROPOSAL:

Tests

  1. Hardcode ReportUtils.isTripRoom() to always return true
  2. Go to any new room
  3. Verify that you see trip room summary, and that it is as per figma image

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari image
MacOS: Desktop
melvin-bot[bot] commented 1 week ago

@shubham1206agra Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

shawnborton commented 1 week ago

This spacing from the first part of your PR comment looks good: image

But further down in your screenshots section, the spacing here looks different: image

rushatgabhane commented 1 week ago

The first image is from figma so it looks good 😄

the spacing here looks different

@shawnborton thank you! Let me fix it to match figma

shubham1206agra commented 1 week ago

@rushatgabhane Bump here.

rushatgabhane commented 1 week ago

@shubham1206agra waiting on newDot to receive backend data - https://expensify.enterprise.slack.com/archives/C05S5EV2JTX/p1715299637326109?thread_ts=1714773700.569819&cid=C05S5EV2JTX

rushatgabhane commented 5 days ago

Updated to match figma

image
shawnborton commented 5 days ago

It's still not quite right. We are using 4px gaps between the three lines. The top line should be 16px tall since our label font uses a 16px line height. The middle line should be 20px tall since our regular font uses a 20px line height. And the bottom line should be 14px tall since our micro font size uses a 14px line height.

rushatgabhane commented 5 days ago

1. Top line lineHeight: 16

image

2. Middle lineHeight: 20

image

3. Bottom line: lineHeight: 14px

image

4. Fixed the gap to be 4px.

image
shawnborton commented 5 days ago

Much better, thank you!