-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Replacing deprecated constants and adding new stories #19563
Replacing deprecated constants and adding new stories #19563
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. |
hey @georgewrmarshall, could you please review this PR? |
Codecov Report
@@ Coverage Diff @@
## develop #19563 +/- ##
===========================================
- Coverage 69.45% 69.43% -0.01%
===========================================
Files 985 985
Lines 37289 37289
Branches 10015 10015
===========================================
- Hits 25896 25891 -5
- Misses 11393 11398 +5 |
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.
Looking great! Made a few small updates. LGTM. Thanks for your contribution. We should get a snaps engineer to review also
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! But like George mentioned, it's good to have a review from snaps team and we can merge it
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.
Blocking until solved!
the font size change looks straightforward, once the other suggestion is resolved we can make changes @georgewrmarshall |
20f698d
5cb0b31
to
e8d640b
Compare
e8d640b
to
e0ab16d
Compare
e0ab16d
to
ef6c415
Compare
ef6c415
to
3d3dfc7
Compare
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! Thanks for your contribution @dhruvv173
Suggestions have been resolved and reviewed by Maarten, another snaps engineer
Explanation
SnapRemoveWarning
andSnapUIMarkdown
ui/components/app/snaps/snap-remove-warning
ui/components/app/snaps/snap-ui-markdown
Screenshots/Screencaps
SnapRemoveWarning
SnapUIMarkdown
Manual Testing Steps
yarn storybook
SnapRemoveWarning
andSnapUIMarkdown
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.