-
Notifications
You must be signed in to change notification settings - Fork 135
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
fix: last backtick should be in inline codeblock when sending a text wrapped with two backticks #565
fix: last backtick should be in inline codeblock when sending a text wrapped with two backticks #565
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
@Antasel your commits aren't signed Can you double check that the email on your SSH key is the same as your git config email |
bd03149
to
f3fd643
Compare
@jasperhuangg now signature is verified |
@Antasel even though the issue is only on Web, I'm realizing that we may want to test this on other platforms, can you provide screen captures on other platforms? |
Screenshots/VideosWebweb-inline-codeblock.mp4Mobile Web - Chromemobile-web-inline-codeblock-two-backticks.mp4Mobile Web - Safarimobile-safari-inline-codeblock-two-backticks.mp4Desktopdesktop-inline-codeblock-two-backticks.mp4iOSios-inline-codeblock-two-backticks.mp4Androidandroid-two-backticks-2023-08-17-160633.mp4@jasperhuangg could you please check ? |
It didn't auto assign me as C+ here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just dummy comment to add me as reviewer so that it will come in my K2. Will do proper review in my morning.
@Antasel you can add the videos to the author's checklist. |
it's done. |
Fixed Issues
$ Expensify/App#23352
Proposal: Expensify/App#23352 (comment)
Tests
QA
same as test
PR Author checklist
toggleReport
and notonIconClick
).Waiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
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)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Web
web-inline-codeblock.mp4
Mobile Web - Chrome
mobile-web-inline-codeblock-two-backticks.mp4
Mobile Web - Safari
mobile-safari-inline-codeblock-two-backticks.mp4
Desktop
desktop-inline-codeblock-two-backticks.mp4
iOS
ios-inline-codeblock-two-backticks.mp4
Android
android-two-backticks-2023-08-17-160633.mp4