Skip to content
This repository has been archived by the owner on Jul 18, 2024. It is now read-only.

refactor: migrate off paragon modal deprecated component #76

Closed
wants to merge 3 commits into from

Conversation

Mashal-m
Copy link
Contributor

Ticket

Migrate off deprecated Paragon components

What has changed?

  • Migrate off paragon modal deprecation in LibraryAuthoringPage.jsx and UserAccessWidget.jsx

@codecov
Copy link

codecov bot commented Oct 11, 2022

Codecov Report

Base: 49.07% // Head: 49.07% // No change to project coverage 👍

Coverage data is based on head (d02024c) compared to base (a640562).
Patch coverage: 60.00% of modified lines in pull request are covered.

❗ Current head d02024c differs from pull request most recent head d0a1891. Consider uploading reports for the commit d0a1891 to get more accurate results

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           
Impacted Files Coverage Δ
...rary-authoring/library-access/UserAccessWidget.jsx 76.31% <55.00%> (ø)
...-authoring/author-library/LibraryAuthoringPage.jsx 87.56% <80.00%> (ø)

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@brian-smith-tcril brian-smith-tcril left a 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!

Comment on lines 118 to 119
<div>
<p>
Copy link
Contributor

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.

Copy link
Contributor Author

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>
Copy link
Contributor

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>

Copy link
Contributor Author

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>
Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved

@brian-smith-tcril
Copy link
Contributor

@Mashal-m the linter errors on this should go away if you rebase (#78 fixed them)

@arbrandes
Copy link
Contributor

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. :)

@arbrandes arbrandes closed this Oct 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants