Skip to content
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

Paragon form component deprecations #843

Closed
wants to merge 24 commits into from

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 edx.org theme.

Screenshots

screenshot-localhost_1991-2022 08 01-19_45_16

@abdullahwaheed abdullahwaheed requested a review from a team August 1, 2022 14:46
@abdullahwaheed abdullahwaheed self-assigned this Aug 1, 2022
@codecov
Copy link

codecov bot commented Aug 1, 2022

Codecov Report

Base: 82.73% // Head: 82.72% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (175b41b) compared to base (93e046d).
Patch coverage: 87.69% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #843      +/-   ##
==========================================
- Coverage   82.73%   82.72%   -0.01%     
==========================================
  Files         353      353              
  Lines        8484     8537      +53     
  Branches     1893     1910      +17     
==========================================
+ Hits         7019     7062      +43     
- Misses       1436     1446      +10     
  Partials       29       29              
Impacted Files Coverage Δ
.../MultipleFileInputField/MultipleFileInputField.jsx 88.23% <ø> (ø)
src/components/FileInput/index.jsx 35.71% <50.00%> (+0.71%) ⬆️
.../LmsConfigurations/CanvasIntegrationConfigForm.jsx 55.76% <57.14%> (+1.05%) ⬆️
...Configurations/BlackboardIntegrationConfigForm.jsx 54.54% <80.00%> (-1.02%) ⬇️
...igurations/SuccessFactorsIntegrationConfigForm.jsx 73.43% <84.61%> (-0.25%) ⬇️
...mlProviderConfiguration/SamlProviderConfigForm.jsx 70.17% <85.71%> (-0.98%) ⬇️
...components/ReportingConfig/ReportingConfigForm.jsx 79.81% <87.17%> (+1.40%) ⬆️
...SamlProviderConfiguration/SamlProviderDataForm.jsx 71.42% <88.23%> (+3.98%) ⬆️
...onfigurations/CornerstoneIntegrationConfigForm.jsx 83.33% <100.00%> (+0.52%) ⬆️
...LmsConfigurations/DegreedIntegrationConfigForm.jsx 84.05% <100.00%> (ø)
... and 6 more

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.

@@ -144,8 +143,14 @@ class FileInput extends React.Component {
</Button>
</>
)}
<Form.Text>{description}</Form.Text>
{hasError && (
<Form.Control.Feedback icon={<Error className="mr-1" />}>
Copy link
Member

Choose a reason for hiding this comment

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

[curious] Why not use the default icon that Form.Control.Feedback uses versus overriding the default invalid icon? Same question applies to all uses of Form.Control.Feedback that passes an <Error />. Because isInvalid is passed to the top-level Form.Group, the default icon that Form.Control.Feedback uses should be the correct icon already.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

previously it was using Error icon, so to make it consistent with previous design i had added this
Screenshot 2022-07-28 at 12 49 14 PM

src/components/ReportingConfig/SFTPDeliveryMethodForm.jsx Outdated Show resolved Hide resolved
src/components/SamlConfiguration/index.jsx Outdated Show resolved Hide resolved
@abdullahwaheed
Copy link
Contributor Author

its outdated. created a new PR #933

@abdullahwaheed abdullahwaheed deleted the abdullah/paragon-form-deprecations branch January 6, 2023 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants