-
Notifications
You must be signed in to change notification settings - Fork 25
refactor: migrate off paragon modal deprecated component #76
Conversation
Codecov ReportBase: 49.07% // Head: 49.07% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #76 +/- ##
=======================================
Coverage 49.07% 49.07%
=======================================
Files 76 76
Lines 1997 1997
Branches 373 373
=======================================
Hits 980 980
Misses 983 983
Partials 34 34
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
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.
This looks great! Very happy to see depreciated components replaced!
Couple little comments in there but overall this is looking really good!
<div> | ||
<p> |
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.
do we need the <div>
and the <p>
here? my assumption would be that the <div>
was needed before because the body wasn't a component, but now that we have <ModalDialog.Body>
it might be redundant.
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.
Resolved
</ModalDialog.Title> | ||
</ModalDialog.Header> | ||
<ModalDialog.Body> | ||
<div> |
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.
same as previous comment about probably not needing this <div>
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.
Resolved
</ModalDialog.Title> | ||
</ModalDialog.Header> | ||
<ModalDialog.Body> | ||
<div> |
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.
another <div>
we probably don't need
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.
Resolved
Thanks for the PR, @Marshal-M! For expediency (we want to get another PR in today), we rebased and merged your code via #79. In other words, you can count this as a successfull contribution. :) |
Ticket
Migrate off deprecated Paragon components
What has changed?