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

Paragon form component deprecations #53

Merged
merged 15 commits into from
Oct 4, 2022

Conversation

abdullahwaheed
Copy link
Contributor

Ticket

Migrate off deprecated Paragon components

What has changed

Updated deprecated ValidationFormGroup to Form.Group, Input to Form.Control and Checkbox to Form. CheckBox.
Tested on openedx theme as well.

@abdullahwaheed abdullahwaheed requested a review from a team July 25, 2022 14:23
@abdullahwaheed abdullahwaheed self-assigned this Jul 25, 2022
…g into abdullahwaheed/paragon-form-deprecations
@codecov
Copy link

codecov bot commented Aug 16, 2022

Codecov Report

Base: 49.06% // Head: 49.07% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (a8bcbb8) compared to base (01e94b1).
Patch coverage: 50.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #53   +/-   ##
=======================================
  Coverage   49.06%   49.07%           
=======================================
  Files          76       76           
  Lines        1985     1997   +12     
  Branches      366      373    +7     
=======================================
+ Hits          974      980    +6     
- Misses        977      983    +6     
  Partials       34       34           
Impacted Files Coverage Δ
src/library-authoring/common/LicenseField.jsx 25.00% <ø> (ø)
...thoring/configure-library/LibraryConfigurePage.jsx 3.44% <0.00%> (-0.22%) ⬇️
...-authoring/author-library/LibraryAuthoringPage.jsx 87.56% <100.00%> (+0.06%) ⬆️
...brary-authoring/course-import/CourseImportPage.jsx 86.36% <100.00%> (+0.64%) ⬆️
...ary-authoring/library-access/LibraryAccessForm.jsx 91.17% <100.00%> (-2.58%) ⬇️

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.

…g into abdullahwaheed/paragon-form-deprecations
…g into abdullahwaheed/paragon-form-deprecations
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.

I tested this locally and it's functioning beautifully!

I did leave a few comments in here, mostly really small cleanup stuff, and a couple questions.

It's wonderful to see depreciated components getting replaced!

src/library-authoring/common/LicenseField.jsx Outdated Show resolved Hide resolved
src/library-authoring/common/LicenseField.jsx Outdated Show resolved Hide resolved
src/library-authoring/common/LicenseField.jsx Outdated Show resolved Hide resolved
src/library-authoring/common/LicenseField.jsx Outdated Show resolved Hide resolved
src/library-authoring/common/index.scss Outdated Show resolved Hide resolved
Comment on lines +182 to +193
const renderOption = option => {
if (option.group) {
return (
<optgroup label={option.label}>
{option.group.map(renderOption)}
</optgroup>
);
}
return (
<option value={option.value} key={option.value}>{option.label}</option>
);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

i noticed there is an identical function in LibraryListPage.jsx, is there any way this could be put somewhere that allows it to be used in both files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renderOption isn't used in LibraryListPage.jsx anymore so I have removed it. Don't think we should make it a util now

@abdullahwaheed abdullahwaheed added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed engineering review needs triage labels Sep 30, 2022
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.

LGTM!

one little comment in there about a commented out line, but even without changing that this looks good to merge

src/library-authoring/common/LicenseField.jsx Outdated Show resolved Hide resolved
@abdullahwaheed abdullahwaheed removed the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label Oct 4, 2022
@abdullahwaheed abdullahwaheed merged commit 73beff6 into master Oct 4, 2022
@abdullahwaheed abdullahwaheed deleted the abdullahwaheed/paragon-form-deprecations branch October 4, 2022 06:20
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