Skip to content

Auto register icons #1499

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 4 commits into from
Jan 16, 2025

Conversation

elenagarrone
Copy link
Contributor

@elenagarrone elenagarrone commented Jan 14, 2025

workerB

Description

This PR contains breaking changes:

  • updates both Brand and UI icons code to automatically register the icons
  • updates svg-to-ts configs to generate a separate file for each svg
  • updates the webpack config to move the new files into the dist
  • updates the new lib folders to .gitignore
  • updates the scripts in the package.json
  • updates the brand icon component Inputs to make sure the halfToneColour and outlineColour are always set
  • removes the svg path from the .lintstagedrc.js as how it worked was not ideal and the icons are now generated when npm run build runs
  • updates all the stories and test app to not register icons anymore
  • removes registerIcon and registerBrandIcon from the registries
  • updated getIcon and getBrandIcon to be renamed to get. They both return a Promise.
  • updated pull_request.yml to run the Build and Test job together. Build is needed before Test because it has to generate the icons sets.

Note
All the changes have been tested within our apps and they work.

Additional test on deployed environment:

  • Accordion with icons loads correctly
    image
  • The icons are lazy loaded correctly
    image

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 added the documentation
  • I have added any new public feature modules to public-api.ts

@elenagarrone elenagarrone requested a review from a team as a code owner January 14, 2025 10:43
@github-actions github-actions bot added the deployed The branch is deployed to GitHub Pages label Jan 14, 2025
@elenagarrone elenagarrone force-pushed the fep-2796-ar-icons branch 2 times, most recently from 80d18e0 to 6b1de2b Compare January 14, 2025 21:57
BREAKING CHANGE: Automatically loads the icon set,
- there is no more need to call `registerIcon`. This function has been removed.
- the getIcon function has been renamed `get` and it returns a Promise.
BREAKING CHANGE: Automatically loads the icon set,
- there is no more need to call `registerBrandIcon`. This function has been removed.
- the getBrandIcon function has been renamed `get` and it returns a Promise.
This is done because 'test' needs the icons to be available
@elenagarrone elenagarrone merged commit 103db0f into Legal-and-General:master Jan 16, 2025
3 checks passed
@elenagarrone elenagarrone deleted the fep-2796-ar-icons branch January 16, 2025 14:09
Copy link
Contributor

🎉 This PR is included in version 13.0.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
deployed The branch is deployed to GitHub Pages released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants