-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Update expensify-common to updated commit hash #8872
Conversation
@aneequeahmad, don't delete the default PR template from the description. Copy and paste it into the description of the PR and mark the checkboxes accordingly: DetailsFixed Issues$ GH_LINK Tests
PR Review ChecklistContributor (PR Author) Checklist
|
@marcochavezf Ahhh yes, added the PR template. Thanks. |
@aneequeahmad I don't see you included the screenshots/videos for the tests 😅
The issue should be linked with the $ at the beginning:
The testing steps are not included in the Also, follow the instructions that are commented in the template: |
@marcochavezf oh yes, I've learned the whole process. Apologies for the back and forth and Thankyou for bearing with me |
Hi @aneequeahmad! No worries, thank you for following the template and for the testing! I found out that on Android we're still having extra space between the first line and the code block though: For this example I typed this text in the same message:
test Could you check why this is happening? |
@marcochavezf Actually i'm unable to run the project atm 403 forbidden error is thrown by api. I raised this issue in slack channel as well but couldn't find any way to fix this issue :( Will look into this issue as soon as my project is in running state |
@aneequeahmad how's it looking? |
@rushatgabhane unfortunately, I can't run the android app. Will run the project on another machine today and will look into it over the weekend. Thanks bdw @rushatgabhane can you repro this android issue ? it was fine on android last time I remember |
@marcochavezf the PR tests well for me on Android. |
@marcochavezf anyway, a more up-to-date commit for I'd suggest that we
What do you think? Is there something else that we should do? |
Oh I see thanks @rushatgabhane @aneequeahmad! Probably I didn't update the npm package properly at that time, but it's great to know it's working fine on Android!
Yup, that sounds like a plan! I'm going to ping QA team to test the issue again |
@marcochavezf Thats great! Its already in production. Lets ask the QA team to do the testing. @rushatgabhane thanks for verifying on android. |
Details
<br/>\<pre\>...\</pre\><br/>
with\<pre\>...\</pre\>
Fixed Issues
$ #8089
Tests
1- Open the app
2- Navigate to any chat with another user.
3- Write a text in one or more line
4- On next line start codeblock.
5- After codeblock ends write some more text(without any extra line)
6- Send message and check there will be no extra space above or below codeblock.
PR Review Checklist
Contributor (PR Author) Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesSTYLE.md
) were followed/** comment above it */
displayName
propertythis
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)QA Steps
1- Open the app
2- Navigate to any chat with another user.
3- Write a text in one or more line
4- On next line start codeblock.
5- After codeblock ends write some more text(without any extra line)
6- Send message and check there will be no extra space above or below codeblock.
7- Test the above steps on all platforms
Screenshots
Web
Mobile Web
Desktop
iOS
Android
Unfortunately rn can't run on android simulator is broken :( Though i tested on real device when fixed but couldn't take screenshot at that time