Skip to content

feat(icon): add time-delay icon #83

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

Merged
merged 2 commits into from
Nov 30, 2020

Conversation

lt83870
Copy link
Contributor

@lt83870 lt83870 commented Nov 27, 2020

Description

Add time-delay icon to Icons folder. Icon provided by @james-hayden-ellis
https://deploy-preview-83--legal-and-general-canopy.netlify.app/?path=/story/components-icon--standard

Checklist:

  • The commit messages follow the convention for this project
  • I have provided an adequate amount of test coverage
  • I have added the functionality to the test app
  • I have provided a story in storybook to document the changes
  • I have provided documentation in the notes section of the story
  • I have added any new public feature modules to public-api.ts
  • I have linked the new component to adobe DSM (if appropriate)

@andy-polhill
Copy link
Contributor

andy-polhill commented Nov 27, 2020

I paired with @lt83870 (Leo) on this one, but I need to pull it down just to verify the commit.

It does also look like something odd happened in the latest version of svg-to-ts.

@andy-polhill
Copy link
Contributor

I've done a bit of checking and I think when pulled in the latest version of svg-to-ts we may have inadvertently exposed a breaking change.

Each icon set now exports..
export type IconNameSubset<T extends Readonly<Icon[]>> = T[number]['name'];

However the name is the same across both the Icon and BrandIcon set, hence the error in the build.

Possible solutions.

  • Make a temporary modification to the generated files
  • Raise an issue or ideally a PR into svg-to-ts
  • Revert the version to get this icon in

@elenagarrone
Copy link
Contributor

I've done a bit of checking and I think when pulled in the latest version of svg-to-ts we may have inadvertently exposed a breaking change.

Each icon set now exports..
export type IconNameSubset<T extends Readonly<Icon[]>> = T[number]['name'];

However the name is the same across both the Icon and BrandIcon set, hence the error in the build.

Possible solutions.

  • Make a temporary modification to the generated files
  • Raise an issue or ideally a PR into svg-to-ts
  • Revert the version to get this icon in

I'd say revert the version and then we can look into this a bit better.

@lt83870 lt83870 requested a review from a team as a code owner November 27, 2020 22:30
@andy-polhill andy-polhill force-pushed the add-time-delay-icon branch 2 times, most recently from 62a44e7 to ec6ba48 Compare November 27, 2020 22:49
@andy-polhill
Copy link
Contributor

I've reverted and pinned the working version of svg-to-ts. I've also raised an issue into svg-to-ts just to see if they think it's a legitimate bug or something we should workaround.

andy-polhill and others added 2 commits November 27, 2020 22:53
An issue was encountered while updating to the latest version of svg-to-ts, this commit pins it to a
specific version. An [issue](nivekcode/svg-to-ts#91) is raised against the
svg-to-ts repo, depending on the outcome we can either workaround it locally or help provide a fix.
@andy-polhill andy-polhill merged commit 917f91f into Legal-and-General:master Nov 30, 2020
@github-actions
Copy link
Contributor

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants