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

[DSS-328] Upload Card - Layout Update #1782

Merged
merged 20 commits into from
Oct 16, 2023
Merged

Conversation

anechol
Copy link
Contributor

@anechol anechol commented Aug 10, 2023

Description

Refactors the Upload Card layout to match with new Figma specs. Descriptions have been added to the React stories for more context. New props have been added to the React component to accommodate the refactor and match the Rails version.

Note: Because of one usage within kp, the inputProps prop will remain in React. There is a PR in kp that will handle updating those versions.

Screenshots

Before After
Screen Shot 2023-08-15 at 4 09 18 PM Screen Shot 2023-08-15 at 4 40 46 PM

Stacked Layout
Screen Shot 2023-08-15 at 4 08 30 PM

Testing in sage-lib

Rails

  • Navigate to Rails component
  • Check that layout matches Figma specs
  • Check that stacked version matches Figma specs

React

  • Navigate to React component
  • Check that layout matches Figma specs
  • Check that stacked version matches Figma specs

Testing in kajabi-products

(BREAKING) Refactors the Upload Card layout to match with new Figma specs. Will affect existing instances of the Upload Card component in kp that utilize Sage classes for styling.

Related

DSS-328

@anechol anechol self-assigned this Aug 10, 2023
@anechol anechol force-pushed the feature/upload-card-update branch from 14e1ed8 to 97fa66c Compare August 15, 2023 21:34
@anechol anechol added documentation Improvements or additions to documentation enhancement New feature or request Do not merge Requirements must be met before merging labels Aug 15, 2023
@anechol anechol marked this pull request as ready for review August 15, 2023 21:41
@anechol anechol requested a review from a team August 15, 2023 21:41
Copy link
Collaborator

@ju-Skinner ju-Skinner left a comment

Choose a reason for hiding this comment

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

Overall looks good. Left a few comments which do not block approval.

@ju-Skinner ju-Skinner requested a review from a team August 16, 2023 15:16
Copy link
Member

@pixelflips pixelflips left a comment

Choose a reason for hiding this comment

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

A couple of small non-blocking comments, but really great work here! Thanks so much for the documentation updates and the tests! 👍🏼


<h4>Actions area</h4>
<p>Use the <code>sage_upload_card_actions</code> slot to replace the default button and display custom components, such as dropdowns.</p>
<p><strong>NOTE:</strong> a file input field and label are <em>included by default in the base component</em>, as seen in the examples above. When applying a custom file input with `sage_upload_card_actions`, set <code>custom_file_input_field</code> to <code>true</code> to remove these defaults.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Should the sage_upload_card_actions be wrapped in <code> to match the others?

],
file_selected: true,
accepted_file_types: ["image/jpg"],
selection_preview: "https://placekitten.com/360",
Copy link
Member

Choose a reason for hiding this comment

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

🐈

<h3 class="t-sage-heading-6">Selected Error State</h3>
<h3>Custom content areas</h3>
<h4>Instructions area</h4>
<p>The <code>sage_upload_card_instructions</code> slot provides an area for extended instructions and custom markup.</p>
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: extra space between 'for extended'

@ju-Skinner ju-Skinner force-pushed the develop branch 2 times, most recently from c85f983 to 39d0349 Compare August 18, 2023 15:09
@ju-Skinner ju-Skinner force-pushed the develop branch 11 times, most recently from b5f45fa to d0f2ff2 Compare August 18, 2023 17:51
@anechol anechol force-pushed the feature/upload-card-update branch 2 times, most recently from b5d7e28 to fa3b177 Compare August 21, 2023 15:06
@anechol anechol force-pushed the feature/upload-card-update branch from fa3b177 to 3ded589 Compare September 8, 2023 14:42
@anechol anechol force-pushed the feature/upload-card-update branch from 3ded589 to 31f5f3d Compare September 19, 2023 15:13
@anechol anechol force-pushed the feature/upload-card-update branch from 31f5f3d to fc18490 Compare September 27, 2023 15:30
@anechol anechol force-pushed the feature/upload-card-update branch from fc18490 to ad36f96 Compare October 16, 2023 15:37
@anechol anechol removed the Do not merge Requirements must be met before merging label Oct 16, 2023
@anechol anechol merged commit 83602ea into develop Oct 16, 2023
4 checks passed
@anechol anechol deleted the feature/upload-card-update branch October 16, 2023 16:57
@anechol anechol mentioned this pull request Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants