-
Notifications
You must be signed in to change notification settings - Fork 18
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
refactor: migrate off modal paragon deprecated component #71
refactor: migrate off modal paragon deprecated component #71
Conversation
Codecov Report
@@ Coverage Diff @@
## main #71 +/- ##
=======================================
Coverage 88.47% 88.47%
=======================================
Files 68 68
Lines 911 911
Branches 247 247
=======================================
Hits 806 806
Misses 98 98
Partials 7 7
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
- Coverage 93.68% 88.57% -5.12%
==========================================
Files 70 68 -2
Lines 1014 910 -104
Branches 279 248 -31
==========================================
- Hits 950 806 -144
- Misses 59 97 +38
- Partials 5 7 +2
☔ View full report in Codecov by Sentry. |
@leangseu-edx, any chance you could review this? @Mashal-m, mind updating the branch with master in the meantime? |
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.
Shouldn't ModalDialog.body
is uppercase?
Anyway, can you use ModalDialogBody
, ModalDialogHeader`.... syntax instead? Since those are import directly and it is less likely to create typo and it is easier to check with the doc on paragon https://paragon-openedx.netlify.app/components/modal/modal-dialog/
3558267
to
2705d40
Compare
According to paragon documentation, we can only import ModaDialog and ModalCloseButton directly. It is mandatory to use header, title, etc like this ModalDialog.Body. |
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 👍
Fair. It seems all of the example are doing that. |
🎉 This PR is included in version 2.16.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
* refactor: migrate off modal paragon deprecated component * refactor: fixed ModalDialog body uppercase * refactor: change button tag
Ticket
Migrate off deprecated Paragon components
What has changed?