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.

Sign up for GitHub

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

Jump to bottom

GC-2963/Add repeat icon #518

Merged
merged 2 commits into from
Jun 17, 2024
Merged

GC-2963/Add repeat icon #518

merged 2 commits into from
Jun 17, 2024

Conversation

haroutrs
Copy link
Contributor

JIRA

Description

  • Adds Repeat icon (for use in dashboard campaignSendType modal)

Reviewer Checklist

This section is to be filled out by reviewers

Testing

  • This code was tested by somebody other than the developer. Do not merge until this has been done.

@haroutrs haroutrs requested a review from a team as a code owner June 17, 2024 18:09
@guardrails GuardRails
Copy link

guardrails bot commented Jun 17, 2024

⚠️ We detected 6 security issues in this pull request:

Vulnerable Libraries (6)
Severity Details
Critical pkg:npm/[email protected] (t) upgrade to: > 8.2.5
High pkg:npm/[email protected] (t) upgrade to: > 3.1.6
Critical pkg:npm/[email protected] (t) upgrade to: > 10.3.1
High pkg:npm/[email protected] (t) upgrade to: > 1.53.0
Critical pkg:npm/[email protected] (t) upgrade to: > 16.8.3
Medium pkg:npm/[email protected] (t) upgrade to: > 8.19.0

More info on how to fix Vulnerable Libraries in JavaScript.

👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

haroutrs
haroutrs commented Jun 17, 2024
View reviewed changes
package-lock.json
"lockfileVersion": 2,
"requires": true,
"packages": {
"": {
"name": "@lob/ui-components",
"version": "2.0.73",
"version": "2.0.74",
Copy link
Contributor Author

@haroutrs haroutrs Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think I updated the version correctly where needed (but please confirm)

Copy link
Contributor

Choose a reason for hiding this comment

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

I have found npm version patch --git-tag-version false to work great

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, thanks! Much easier 😄

@aws-amplify-us-west-2 AWS Amplify (us-west-2)
Copy link

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-518.d11k469e311m4w.amplifyapp.com

haroutrs
haroutrs commented Jun 17, 2024
View reviewed changes
src/components/Icon/svgs/Repeat.svg Outdated
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks correct here, but does not look right on the preview..

Screenshot 2024-06-17 at 11 15 51 AM

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm...
Try this SVG

<svg>
<path d="M4.58332 14.5834H3.33332V5.41672H9.83332L9.16666 6.07505C9.00974 6.23087 8.92116 6.44264 8.92032 6.66377C8.91957 6.88491 9.00666 7.0973 9.16249 7.25422C9.31832 7.41114 9.53007 7.49974 9.75124 7.50052C9.97232 7.50129 10.1847 7.4142 10.3417 7.25839L12.425 5.17505C12.5031 5.09759 12.5651 5.00542 12.6074 4.90387C12.6497 4.80232 12.6715 4.69339 12.6715 4.58339C12.6715 4.47338 12.6497 4.36445 12.6074 4.2629C12.5651 4.16135 12.5031 4.06919 12.425 3.99172L10.3417 1.90839C10.1855 1.75318 9.97432 1.66606 9.75416 1.66606C9.53399 1.66606 9.32282 1.75318 9.16666 1.90839C9.08857 1.98585 9.02657 2.07802 8.98424 2.17957C8.94191 2.28112 8.92016 2.39004 8.92016 2.50005C8.92016 2.61006 8.94191 2.71899 8.98424 2.82054C9.02657 2.92209 9.08857 3.01425 9.16666 3.09172L9.82499 3.75005H2.49999C2.27897 3.75005 2.06701 3.83785 1.91073 3.99413C1.75446 4.15041 1.66666 4.36237 1.66666 4.58339V15.4168C1.66666 15.6378 1.75446 15.8497 1.91073 16.006C2.06701 16.1623 2.27897 16.2501 2.49999 16.2501H4.58332C4.80434 16.2501 5.0163 16.1623 5.17258 16.006C5.32886 15.8497 5.41666 15.6378 5.41666 15.4168C5.41666 15.1957 5.32886 14.9838 5.17258 14.8275C5.0163 14.6712 4.80434 14.5834 4.58332 14.5834ZM17.5 3.75005H15.4167C15.1957 3.75005 14.9837 3.83785 14.8274 3.99413C14.6712 4.15041 14.5833 4.36237 14.5833 4.58339C14.5833 4.8044 14.6712 5.01636 14.8274 5.17264C14.9837 5.32892 15.1957 5.41672 15.4167 5.41672H16.6667V14.5834H9.69166L10.35 13.9251C10.4281 13.8476 10.4901 13.7554 10.5324 13.6538C10.5747 13.5523 10.5965 13.4434 10.5965 13.3334C10.5965 13.2233 10.5747 13.1144 10.5324 13.0129C10.4901 12.9113 10.4281 12.8192 10.35 12.7418C10.1938 12.5865 9.98266 12.4994 9.76249 12.4994C9.54232 12.4994 9.33116 12.5865 9.17499 12.7418L7.09166 14.8251C7.01355 14.9025 6.95156 14.9947 6.90925 15.0963C6.86694 15.1978 6.84516 15.3067 6.84516 15.4168C6.84516 15.5268 6.86694 15.6357 6.90925 15.7372C6.95156 15.8388 7.01355 15.9309 7.09166 16.0084L9.17499 18.0918C9.33191 18.2475 9.54432 18.3347 9.76541 18.3338C9.98657 18.3331 10.1983 18.2445 10.3542 18.0876C10.51 17.9307 10.5971 17.7183 10.5963 17.4971C10.5955 17.276 10.5069 17.0642 10.35 16.9084L9.69166 16.2501H17.5C17.721 16.2501 17.933 16.1623 18.0892 16.006C18.2455 15.8497 18.3333 15.6378 18.3333 15.4168V4.58339C18.3333 4.36237 18.2455 4.15041 18.0892 3.99413C17.933 3.83785 17.721 3.75005 17.5 3.75005Z" fill="currentColor"/>
</svg>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Giving it a go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, looks good now

Screenshot 2024-06-17 at 11 34 59 AM

For my own learning, how did you fix it? I saw there are docs for resizing in the readme but thought that may be outdated

Copy link
Contributor

@NateWaldschmidt NateWaldschmidt Jun 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I copied the one in figma and then resized it in figma to be 20 x 20 then copied the SVG into the icon directory

I tried to document it here but it may need some improvements, feel free to update it if there is something missing
https://ui-components.lob.com/?path=/docs/components-icon--docs#adding-icons

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it, good to know. Thanks Nate!

@guardrails GuardRails
Copy link

guardrails bot commented Jun 17, 2024

⚠️ We detected 6 security issues in this pull request:

Vulnerable Libraries (6)
Severity Details
Critical pkg:npm/[email protected] (t) upgrade to: > 8.2.5
High pkg:npm/[email protected] (t) upgrade to: > 3.1.6
Critical pkg:npm/[email protected] (t) upgrade to: > 10.3.1
High pkg:npm/[email protected] (t) upgrade to: > 1.53.0
Critical pkg:npm/[email protected] (t) upgrade to: > 16.8.3
Medium pkg:npm/[email protected] (t) upgrade to: > 8.19.0

More info on how to fix Vulnerable Libraries in JavaScript.

👉 Go to the dashboard for detailed results.

📥 Happy? Share your feedback with us.

@haroutrs haroutrs merged commit 270af90 into main Jun 17, 2024
4 of 5 checks passed
@haroutrs haroutrs deleted the GC-2963/add-repeat-icon branch June 17, 2024 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NateWaldschmidt NateWaldschmidt NateWaldschmidt approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@haroutrs @NateWaldschmidt