-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Part of #17670: Replace Typography with Text component in: callout.js #18912
Part of #17670: Replace Typography with Text component in: callout.js #18912
Conversation
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
Co-authored-by: Danica Shen <zhaodanica@gmail.com>
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.
Need to fix linting issue otherwise LGTM
Hey @georgewrmarshall , I have added the test tokens from the test dApp. |
…xt-component-in-callout.js
Hey @PrgrmrHarshShukla, just wanted to check in. It looks like this PR is almost there. Did you still want to work on it? If so, that's great! Let us know if you need any assistance or if there are any blockers. If you're unable to continue working on it or haven't had a chance to address it, no worries! I can take it from here and carry it forward. Cheers! |
Hey @georgewrmarshall , Also , due to an unknown issue, I was not able to get the extension running locally, so I was not able to take any screenshots. I did not want to trouble you. Now, I am going to fork this again and delete the old fork. |
Hey @PrgrmrHarshShukla, looks like you need to run |
Hey @PrgrmrHarshShukla, thanks for all your effort with this PR I am going to close it as it has become stale. Please feel free to open another PR in future. Thanks for your contributions thus far! |
Hey @georgewrmarshall , Equipped with all the experiences and learnings from the PRs till date, Regards, |
I have read and agree to the Contributor License Agreement (CLA). |
…xt-component-in-callout.js
…xt-component-in-callout.js
b128a22
Codecov Report
@@ Coverage Diff @@
## develop #18912 +/- ##
===========================================
- Coverage 70.45% 70.45% -0.01%
===========================================
Files 985 985
Lines 38370 38370
Branches 10047 10047
===========================================
- Hits 27033 27031 -2
- Misses 11337 11339 +2
|
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.
LGTM!
Explanation
This PR is a part of #17670 Replace Typography with Text component for:
ui\components\ui\callout\callout.js
Screenshots/Screencaps
Before
After
Manual Testing Steps
Pre-merge author checklist
Pre-merge reviewer checklist
If further QA is required (e.g. new feature, complex testing steps, large refactor), add the
Extension QA Board
label.In this case, a QA Engineer approval will be be required.