-
-
Notifications
You must be signed in to change notification settings - Fork 499
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
fix: made updates to import partners modal #3600
Conversation
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.
@DariBerrie loving the additional fixes around the main issue here! One comment and one question.
app/assets/stylesheets/custom.scss
Outdated
} | ||
#csvImportModal .modal-header { | ||
flex-direction: row; | ||
justify-content: center; |
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.
Spacing is off here.
<li> | ||
<%= f.file_field :file, accept: 'text/csv', class: 'form-control-file', style: "margin: auto;" %> | ||
</li> | ||
<li style="margin-bottom: 27px;">Then click the "Import CSV" button to import your <%= import_type.downcase %>.</li> | ||
<li style="margin-bottom: 27px;">Then click the "Import CSV" button to import your partners.</li> |
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.
Wouldn't this show the same text for other import CSV modals though?
@@ -37,7 +37,7 @@ | |||
</div><!-- / row --> | |||
</div><!-- /.modal-body --> | |||
<div class="modal-footer"> | |||
<small>Note: You're only able to run the import one time to prevent accidental imports or writing over existing locations!</small> | |||
<small>Note: You're only able to run the import one time to prevent accidental imports or writing over existing partners!</small> |
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.
Ditto here?
@dorner Let me know if this looks okay. I can make additional changes if needed :) |
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.
Looks good to me! We need to fix the tests (there's an issue with knapsack). :(
Working now! Woot! |
Awesome! I was worried when I saw those tests fail 😅 |
Resolves #3585
Description
I updated the text on the Import Partners modal to improve clarity for users. I also made some small style changes to the text like using capitalized CSV for all mentions, adjusting the alignment of text, and matching button sizes for both columns.
I believe all CSS changes are unique to the modal, since the _csv_import_modal.html.erb only uses partners terminology. If it is used elsewhere and may affect the modal's appearance, I can check what would be best to standardize the modals.
Let me know what you think! Would welcome any feedback!
List any dependencies that are required for this change. (gems, js libraries, etc.) None.
Type of change
How Has This Been Tested?
Only made text changes and CSS changes that can be seen on the modal page. Screenshots provided below.
Screenshots