fix: offline deleted heading styles and link padding #19545

Nikhil-Vats · 2023-05-24T16:51:36Z

Details

This PR fixes the following issues -

Deleted headings in offline mode looked like underlined instead of strikethrough.
When only a link is added in the header text, the app adds extra padding above the link which has the cursor as a pointer.

The first one was a regression from #16526, this PR fixes that issue using another way.

Fixed Issues

$ #18658
$ #17488
$ #18658 (comment)

Tests

For testing #17488 (padding above header links) -

Open the app in Chrome/safari on your desktop.
Open a chat.
Send any link as header text. eg: # google.com
Verify that there is no extra padding above links and that links are only clickable (with the cursor as a pointer) when the cursor is over the text not above it.

You can also follow the same steps on the desktop app on MacOS to verify that it is working successfully there as well.

For testing #16526 (cursor shown in empty space for links that span multiple lines) -

Post a long hyperlink as a comment.
e.g link : Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer ornare dignissim nunc, eleifend molestie dui tempus non. Proin semper eu metus sit amet feugiat. Sed turpis augue, pellentesque sed accumsan ornare, malesuada in ligula. Aliquam porta condimentum varius. Morbi id lorem felis.
Hover over the hyperlink after posting the comment.
On web and desktop platforms pointer cursor should be shown only when the mouse cursor is over the hyperlink and not when it is over the empty space near the line endings.
Regular cursor should be shown while hovering over the empty area next to the hyperlink.
On other platforms, the comment should be shown exactly as it was rendered earlier.
All other comment types should render exactly the same as before.
Same behaviour is applicable for links posted as a reply to a comment via email.

Verify that no errors appear in the JS console

Offline tests

Issue #18658 must be tested in offline mode. Follow the steps below -

Open the app in Chrome on your desktop.
Go to a chat and add a heading comment. eg: # Example
Go offline by switching off the connection.
Delete the message.
Verify that the message is shown with a strikethrough style (line over the message) and not underlined.

Note -

Repeat the steps above for the following messages (feel free to add more) -
Messages are separated by a new empty line.

# text header

# header with link.com

# header with emoji 😇

# 😇😇

> # header inside quote with [link](google.com)

`# try header in code`

\```
multiline code (remove \ before three backticks above and below)
😅
link.com
\```

😋😆😇😇

[normal link](google.com)

Also repeat the steps for different attachments - images, PDFs, files without preview like CSVs, docs. Upload them in online mode, go offline and delete them.
a. After deleting attachments like images, PDFs, GIFs which have preview, their opacity should decrease and user should not be able to open them.
b. For attachments without previews like CSV, docs, etc. A strikethrough should be shown similar to text comments.
Add any attachments in offline mode. App should keep showing "Uploading attachment...". Delete the attachment. App should show a strikethrough over the text.

You can test this behaviour on a mobile browser, desktop browser, MacOS desktop app, iOS app, android app, etc using the same steps.

QA Steps

Follow the same steps listed in the section above. Expand this to see the same steps.

For testing #17488 (padding above header links) -

Open the app in Chrome/safari on your desktop.
Open a chat.
Send any link as header text. eg: # google.com
Verify that there is no extra padding above links and that links are only clickable (with the cursor as a pointer) when the cursor is over the text not above it.

You can also follow the same steps on the desktop app on MacOS to verify that it is working successfully there as well.

For testing #16526 (cursor shown in empty space for links that span multiple lines) -

Post a long hyperlink as a comment.
e.g link : Lorem ipsum dolor sit amet, consectetur adipiscing elit. Integer ornare dignissim nunc, eleifend molestie dui tempus non. Proin semper eu metus sit amet feugiat. Sed turpis augue, pellentesque sed accumsan ornare, malesuada in ligula. Aliquam porta condimentum varius. Morbi id lorem felis.
Hover over the hyperlink after posting the comment.
On web and desktop platforms pointer cursor should be shown only when the mouse cursor is over the hyperlink and not when it is over the empty space near the line endings.
Regular cursor should be shown while hovering over the empty area next to the hyperlink.
On other platforms, the comment should be shown exactly as it was rendered earlier.
All other comment types should render exactly the same as before.
Same behaviour is applicable for links posted as a reply to a comment via email.

Offline tests

Issue #18658 must be tested in offline mode. Follow the steps below -

Open the app in Chrome on your desktop.
Go to a chat and add a heading comment. eg: # Example
Go offline by switching off the connection.
Delete the message.
Verify that the message is shown with a strikethrough style (line over the message) and not underlined.

Note -

Repeat the steps above for the following messages (feel free to add more) -
Messages are separated by a new empty line.

# text header

# header with link.com

# header with emoji 😇

# 😇😇

> # header inside quote with [link](google.com)

`# try header in code`

\```
multiline code (remove \ before three backticks above and below)
😅
link.com
\```

😋😆😇😇

[normal link](google.com)

Also repeat the steps for different attachments - images, PDFs, files without preview like CSVs, docs. Upload them in online mode, go offline and delete them.
a. After deleting attachments like images, PDFs, GIFs which have preview, their opacity should decrease and user should not be able to open them.
b. For attachments without previews like CSV, docs, etc. A strikethrough should be shown similar to text comments.
Add any attachments in offline mode. App should keep showing "Uploading attachment...". Delete the attachment. App should show a strikethrough over the text.

You can test this behaviour on a mobile browser, desktop browser, MacOS desktop app, iOS app, android app, etc using the same steps.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Screenshot for #18658

Video for #16526 and #17488

Screen.Recording.2023-05-24.at.10.44.40.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-05-25.at.1.30.24.AM.mov

Mobile Web - Safari

Screenshot for #18658

Video for #16526

Screen.Recording.2023-05-24.at.11.22.00.PM.mov

Desktop

Screenshot for #18658

Video for #16526 and #17488

Screen.Recording.2023-05-24.at.10.54.16.PM.mov

iOS

Screen.Recording.2023-05-25.at.12.57.59.AM.mov

Android

Screenshot for #18658

Video for #16526 and #17488

Screen.Recording.2023-05-25.at.10.54.42.AM.mov

melvin-bot · 2023-05-24T16:51:42Z

@amyevans @0xmiroslav One of you needs to 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]

github-actions · 2023-05-24T16:51:49Z

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Nikhil-Vats · 2023-05-24T16:53:35Z

I have read the CLA Document and I hereby sign the CLA

Nikhil-Vats · 2023-05-24T21:07:45Z

src/components/PressableWithSecondaryInteraction/index.js

                onPressIn={this.props.onPressIn}
                onLongPress={this.props.onSecondaryInteraction ? this.executeSecondaryInteraction : undefined}
                onPressOut={this.props.onPressOut}
                onPress={this.props.onPress}
                ref={(el) => (this.pressableRef = el)}
                // eslint-disable-next-line react/jsx-props-no-spreading
                {...defaultPressableProps}
+                style={StyleUtils.combineStyles(this.props.style, this.props.inline && styles.dInline)}


added this at the end, since defaultPressableProps were overwriting the style prop when it was in the first line as defaultPressableProps have this.props.style as one of the keys. Here, we add display inline to default props, if inline is true.

@Nikhil-Vats This led to regression for menu items on the Web and Desktop.

web

desktop

Another regression on the copy url clipboard popover.

DEV

STAGING

@mollfpr both fixed. I was not handling function type style props. Updated the PR.

Nikhil-Vats · 2023-05-24T21:08:55Z

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

@@ -48,6 +48,7 @@ const BaseAnchorForCommentsOnly = (props) => {
    return (
        <PressableWithSecondaryInteraction
            inline
+            style={[styles.cursorDefault, {fontSize: props.style.fontSize}]}


not setting display to inline here as it will pass the style to the native component as well which doesn't support the inline value.

parasharrajat · 2023-05-24T21:14:49Z

src/components/HTMLEngineProvider/defaultViewProps/index.web.js

-    // child elements to be block elements even when they have display inline added to them.
-    // This will affect elements like <a> which are inline by default.
-    style: [styles.dBlock, styles.userSelectText],
+    style: [styles.userSelectText],


I see we are reverting to this change. But where are we fixing the original issue the change was fixing?

By passing the cursorDefault style in PressableWithSecondaryInteraction in BaseAnchorForCommentsOnly.

parasharrajat · 2023-05-24T23:32:09Z

So you are basically solving two issues #18658
and #17488.

#17488 will be solved soon.

parasharrajat · 2023-05-24T23:34:05Z

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

@@ -48,6 +48,7 @@ const BaseAnchorForCommentsOnly = (props) => {
    return (
        <PressableWithSecondaryInteraction
            inline
+            style={[styles.cursorDefault, {fontSize: props.style.fontSize}]}


This will not solve #16526 completely. It also creates bugs. You will find more details on the issue discussion. I don't remember.

I tested all three issues and they were fixed on the respective platforms. I will go through the issue discussion on #16526 and check if there are any bugs.

@parasharrajat I have read through your concerns on #16526 and tested out all kinds of message types - Attachments (with and without previews), and various markdown types like header, italic, bold, quote, code, code blocks, links, email comments etc. and it works as expected.

BaseAnchorForCommentsOnly is used only for rendering messages. Feel free to let me know if I missed anything or if you want me to test any particular use case/issue more.

mollfpr · 2023-05-29T19:27:57Z

@Nikhil-Vats Sorry I didn't get the time to test this, but I'll try to complete the review in the morning.

mollfpr · 2023-05-30T10:49:53Z

@Nikhil-Vats Is this expected in iOS to open the link when clicking on the space?

Also, the emoji got cut out in this PR but not in the latest main.

This PR

Current main

Nikhil-Vats · 2023-05-30T13:33:30Z

@Nikhil-Vats Is this expected in iOS to open the link when clicking on the space?

Also, the emoji got cut out in this PR but not in the latest main.

@mollfpr The links on iOS do open when we click on the space. This happens on the prod app and the main branch as well. The link padding/click issues that this PR is supposed to fix are only for the web environment so I think it is expected behaviour on iOS.

Regarding, the emoji issue. It exists on the main branch and the prod app as well but the emoji should be part of the heading/code (code with 😀). In your screenshot for the main branch, the emoji is not cut because they are not part of the heading/code.

Try these messages on main branch/the app on your phone -

# header with 😀
`code 😀`

It is a known issue for emojis in markdown (italics, heading, code, etc.) - https://expensify.slack.com/archives/C01GTK53T8Q/p1677781537939829

mollfpr · 2023-05-30T15:25:21Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

19545.Web.2.mov

Mobile Web - Chrome

19545.mWeb.Chrome.mov

19545.mWeb.Chrome.2.mov

Mobile Web - Safari

19545.mWeb.Safari.mp4

19545.mWeb.Safari.2.mp4

Desktop

19545.Desktop.2.mov

iOS

Simulator.Screen.Recording.-.iPhone.14.-.2023-05-30.at.22.29.23.mp4

Simulator.Screen.Recording.-.iPhone.14.-.2023-05-30.at.22.33.01.mp4

Android

19545.Android.2.mov

mollfpr · 2023-05-30T15:37:48Z

For attachments without previews like CSV, docs, etc. A strikethrough should be shown similar to text comments.

@Nikhil-Vats The attachment text strikethrough is only for web, no?

Nikhil-Vats · 2023-05-30T16:23:36Z

@Nikhil-Vats The attachment text strikethrough is only for web, no?

@mollfpr yes, the behaviour on phone for all attachments is just reduced opacity.

mollfpr

@Nikhil-Vats Really great test flow presentation 👍

All the objection I found is already address and seems the test step it's covered enough. I'll go a head approve this PR.

All yours @cristipaval @parasharrajat

parasharrajat · 2023-05-30T18:53:35Z

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

@@ -48,6 +48,7 @@ const BaseAnchorForCommentsOnly = (props) => {
    return (
        <PressableWithSecondaryInteraction
            inline
+            style={[styles.cursorDefault, {fontSize: props.style.fontSize}]}


@mollfpr Should we move the inline styles from here?

Hey @parasharrajat, can you please tell me what you mean by this? Do you mean moving the inline prop or the style prop?

Please use Styleutils here. Inline styles are not allowed. https://github.com/Expensify/App/blob/main/contributingGuides/STYLING.md#inline-styles

@parasharrajat Thanks a lot. Updated.

Hey @parasharrajat, friendly bump! 😄

Nikhil-Vats · 2023-06-02T12:36:55Z

Hey @mollfpr, do we need any more approvals or are we good to go?

Nikhil-Vats · 2023-06-05T14:23:54Z

Hey @twisterdotcom @mollfpr @cristipaval, friendly bump! Can we merge this?

mollfpr · 2023-06-05T14:26:04Z

I tend to wait for @parasharrajat final review.

mollfpr · 2023-06-07T03:17:22Z

Bump @parasharrajat

mollfpr · 2023-06-08T06:26:36Z

@cristipaval I think we can merge this. I see that all the concerns have already been addressed.

…line_mode

Nikhil-Vats · 2023-06-08T14:34:11Z

Updated the PR after merge conflicts. New test proofs -

Web

For this issue, link padding issue, multiline comment issue -

Screen.Recording.2023-06-08.at.7.51.54.PM.mov

For attachment issue (attachments are downloadable when we click on right of the button) -

web2.mov

Desktop

For this issue, link padding issue, multiline comment issue -

desktop4.mov

For attachment issue (attachments are downloadable when we click on right of the button) -

desktop2.mov

mWeb chrome android

For link padding issue, multiline comment issue -

mWeb.chrome.android.1.mov

For this issue and attachment issue (attachments are downloadable when we click on right of the button) -

mWeb.chrome.android.2.mov

mWeb safari iOS

For this issue -

mWeb.safari.ios.2.mov

For link padding issue, multiline comment issue and attachment issue (attachments are downloadable when we click on right of the button) -

mWeb.safari.ios.1.mov

mWeb.safari.ios.3.mov

android

For this issue -

For link padding issue, multiline comment issue and attachment issue (attachments are downloadable when we click on right of the button) -

android.1.mov

iOS

For this issue -

For link padding issue, multiline comment issue and attachment issue (attachments are downloadable when we click on right of the button) -

ios1.mov

Nikhil-Vats · 2023-06-08T16:53:24Z

@mollfpr @cristipaval There were merge conflicts since PR for #19717 was merged in main. This issue was also a regression from the same PR that caused this one so they were closely related.

So, I talked with @bernhardoj who worked on the other issue and also added test proofs for their issue (attachments and images were clickable in empty space on right) in my latest comment.

So my latest comment includes test proofs for -

this issue - [HOLD for payment 2023-06-30] [$1000] Chat - Deleted messages with heading style not strike-through in offline #18658
attachments clickable issue - Add terms to signin/settings pages #1971
the original issue that created 1 and 2 - [Pay May 15][$2000] In a long link that split into several lines, the pointer cursor is shown in the empty space #16526
links in header have extra padding which is clickable - [HOLD for payment 2023-06-30] [Hold 18658][$1000] When only link is added in header text, app adds extra padding above link which has cursor as pointer #17488

Nikhil-Vats · 2023-06-08T16:55:35Z

src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.js

@@ -60,7 +60,7 @@ const ImageRenderer = (props) => {
                >
                    {({show}) => (
                        <PressableWithoutFocus
-                            style={styles.noOutline}
+                            styles={styles.noOutline}


Also, there was a typo here since the name of prop for PressableWithoutFocus is styles not style.

App/src/components/PressableWithoutFocus.js

Line 17 in 213ba55

styles: PropTypes.arrayOf(PropTypes.object),

I noticed this while reviewing the PR for #19717 so I am fixing it here.

I get this console error:

This is getting fixed in one of the PR.

@parasharrajat can you link the PR if possible?
Because if that’s a big one and will take time to be merged, I can raise a PR and fix this sooner.

@mollfpr @cristipaval
styles has to be an array here and I passed an object.

I made this typo while fixing an issue created in a related PR. Shall I create a new PR for this so that it is fixed before QA testing? Otherwise if the other PR that @parasharrajat is talking about takes a lot of time then this PR will be blocked unnecessarily.

Feel free to drop another PR to fix this. Please follow this https://github.com/Expensify/App/pull/20202/files#r1224269258

cristipaval

Thanks @Nikhil-Vats for keeping us updated and for the rigorous testing!

OSBotify · 2023-06-08T17:21:44Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2023-06-13T00:33:26Z

🚀 Deployed to staging by https://github.com/cristipaval in version: 1.3.27-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

OSBotify · 2023-06-13T01:07:35Z

🚀 Deployed to staging by https://github.com/cristipaval in version: 1.3.27-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

youssef-lr · 2023-06-13T15:25:44Z

src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

@@ -53,6 +54,7 @@ const BaseAnchorForCommentsOnly = (props) => {
    return (
        <PressableWithSecondaryInteraction
            inline
+            style={[styles.cursorDefault, StyleUtils.getFontSizeStyle(props.style.fontSize)]}


Any reason why we added this? Removing this it fixes this issue.

This is caused by this line but reverting it is not the solution because we were adding the pointer cursor not to the link but to its parent which showed the pointer at places around the link -

Screen.Recording.2023-06-13.at.9.46.29.PM.mov

The root cause for this issue is that for some reasons links like https://staging.new.expensify.com/random_text_here are rendered in div rather than a which is wrong.

Screen.Recording.2023-06-13.at.9.48.01.PM.mov

So, we need to figure out why links are rendered in div and change it to a to fix it the right way.

Internal links need special handling before opening them then they are opened via onPress handlers. We don't want them to open immediately so a tag is disabled on them. Only when href is passed AnchorRenderer are rendered as a. THis was the issue I was mentioning #19545 (comment) when I said that this will not work for the #16526. For the same reason, I didn't select a cursor-based solution on that solution. You can see that it is presented as a proposal on that.

Unfortunately, this didn't come to my mind before.

Thanks for weighing in @parasharrajat. I think helps us further to figure out a fix?

@mollfpr @cristipaval This PR was reverted because of the special handling of internal links explained by @parasharrajat above.

I found a solution -
basically for internal links since they are rendered as div we can explicitly set cursor to pointer on line 71. For normal links it is added by default so the only behaviour we are changing is for internal links by adding pointer cursor -
(Text below uses div for internal links and a for other/normal links)

App/src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly.js

Lines 68 to 71 in 6e8435e

<Tooltip text={props.href}>

<Text

ref={(el) => (linkRef = el)}

style={StyleSheet.flatten([props.style, defaultTextStyle])}

Need to change line 71 to -

style={StyleSheet.flatten([props.style, defaultTextStyle, styles.cursorPointer])}

This would solve all the related issues without regression. I tested on local all issues that were related to my issue like #18658, #17488, #16526, are fixed.

Result -

Screen.Recording.2023-06-13.at.11.01.48.PM.mov

OSBotify · 2023-06-14T21:23:42Z

🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.27-7 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

fix: offline deleted heading styles and link padding

f1b3f25

Nikhil-Vats requested a review from a team as a code owner May 24, 2023 16:51

melvin-bot bot requested review from 0xmiros and amyevans and removed request for a team May 24, 2023 16:51

fix issue with display inline being passed to native

0519582

Nikhil-Vats commented May 24, 2023

View reviewed changes

parasharrajat reviewed May 24, 2023

View reviewed changes

0xmiros mentioned this pull request May 25, 2023

[HOLD for payment 2023-06-30] [Hold 18658][$1000] When only link is added in header text, app adds extra padding above link which has cursor as pointer #17488

Closed

6 tasks

Add support for parsing styles as function

43ae959

Nikhil-Vats requested review from parasharrajat and mollfpr May 26, 2023 00:11

mollfpr previously approved these changes May 30, 2023

View reviewed changes

parasharrajat reviewed May 30, 2023

View reviewed changes

Use StyleUtils for fontSize

79a9cd5

Nikhil-Vats dismissed mollfpr’s stale review via 79a9cd5 May 30, 2023 20:00

Nikhil-Vats requested review from parasharrajat and mollfpr May 30, 2023 20:02

mollfpr approved these changes Jun 1, 2023

View reviewed changes

amyevans requested a review from cristipaval June 1, 2023 15:19

cristipaval previously approved these changes Jun 2, 2023

View reviewed changes

Nikhil-Vats mentioned this pull request Jun 8, 2023

Fix link is shown in a separate line #20440

Merged

57 tasks

Nikhil-Vats added 2 commits June 8, 2023 19:33

Merge branch 'main' into issue/18658_fix_deleted_heading_style_in_off…

69dc50a

…line_mode

Fix issue in header links after merge conflicts

8df3eea

Nikhil-Vats dismissed cristipaval’s stale review via 8df3eea June 8, 2023 14:16

bernhardoj mentioned this pull request Jun 8, 2023

Reattempt to fix multiline comment link show pointer cursor in the empty space line #20114

Merged

57 tasks

Nikhil-Vats commented Jun 8, 2023

View reviewed changes

cristipaval approved these changes Jun 8, 2023

View reviewed changes

cristipaval merged commit 092819f into Expensify:main Jun 8, 2023

Nikhil-Vats mentioned this pull request Jun 12, 2023

17046 — migrate PressableWithotFocus to GenericPressable #20202

Merged

57 tasks

OSBotify mentioned this pull request Jun 12, 2023

Deploy Checklist: New Expensify 2023-06-12 #20640

Closed

71 tasks

situchan mentioned this pull request Jun 13, 2023

[$1000] Sigin - Console error when opening a chat #20448

Closed

6 tasks

youssef-lr mentioned this pull request Jun 13, 2023

[HOLD for payment 2023-06-23] [HOLD for payment 2023-06-21] Web - Chat - Cursor on staging link is not pointer #20669

Closed

6 tasks

youssef-lr reviewed Jun 13, 2023

View reviewed changes

This was referenced Jun 13, 2023

[$1000] Attachment downloaded when user clicked outside the attachment button #19717

Closed

Revert "fix: offline deleted heading styles and link padding" #20695

Merged

kbecciv mentioned this pull request Jun 14, 2023

Web - Chat - Extra padding above header link and cursor displays as a hand over extra padding #20749

Closed

6 tasks

	<Tooltip text={props.href}>
	<Text
	ref={(el) => (linkRef = el)}
	style={StyleSheet.flatten([props.style, defaultTextStyle])}

fix: offline deleted heading styles and link padding #19545

fix: offline deleted heading styles and link padding #19545

Conversation

Nikhil-Vats commented May 24, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

Offline tests

PR Author Checklist

Screenshots/Videos

melvin-bot bot commented May 24, 2023

github-actions bot commented May 24, 2023 • edited Loading

Nikhil-Vats commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nikhil-Vats May 25, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat commented May 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mollfpr commented May 29, 2023

mollfpr commented May 30, 2023

Nikhil-Vats commented May 30, 2023 • edited Loading

mollfpr commented May 30, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

mollfpr commented May 30, 2023

Nikhil-Vats commented May 30, 2023

mollfpr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat May 30, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nikhil-Vats commented Jun 2, 2023

Nikhil-Vats commented Jun 5, 2023 • edited Loading

mollfpr commented Jun 5, 2023

mollfpr commented Jun 7, 2023

mollfpr commented Jun 8, 2023

Nikhil-Vats commented Jun 8, 2023 • edited Loading

Nikhil-Vats commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat Jun 12, 2023 • edited Loading

Choose a reason for hiding this comment

cristipaval left a comment

Choose a reason for hiding this comment

OSBotify commented Jun 8, 2023

OSBotify commented Jun 13, 2023

OSBotify commented Jun 13, 2023

youssef-lr Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat Jun 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OSBotify commented Jun 14, 2023

Nikhil-Vats commented May 24, 2023 •

edited

Loading

github-actions bot commented May 24, 2023 •

edited

Loading

Nikhil-Vats May 25, 2023 •

edited

Loading

Nikhil-Vats commented May 30, 2023 •

edited

Loading

mollfpr commented May 30, 2023 •

edited

Loading

parasharrajat May 30, 2023 •

edited

Loading

Nikhil-Vats commented Jun 5, 2023 •

edited

Loading

Nikhil-Vats commented Jun 8, 2023 •

edited

Loading

parasharrajat Jun 12, 2023 •

edited

Loading

youssef-lr Jun 13, 2023 •

edited

Loading

parasharrajat Jun 13, 2023 •

edited

Loading