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

Remove unused renderSvg, ofnDisableScroll, integer, ofnScrollTo angular directives #12979

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cillian
Copy link
Contributor

@cillian cillian commented Nov 15, 2024

What? Why?

This partially addresses #9477 and removes four unused angular directives.

  • The renderSvg directive was introduced in bc93ce5 and seems to have been removed in Remove taxon icons #5713
  • The ofnDisableScroll directive was introduced in 22aaa24 and seems to have been removed in 59c13b9
  • The integer directive was introduced in 0895bd8 and seems to have been removed in 01c4882
  • The ofnScrollTo directive was introduced in de369d9 and seems to have been removed in 427f535

What should we test?

Confirm the directives aren't called anywhere e.g.

And on staging:

  • Browse through shops and checkout.
  • Everything should look as usual.

Release notes

  • Technical changes only

Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Great, and thanks for thoroughly investigating each case 🏅

Comment on lines -37 to -40
svg {
path {
fill: $disabled-dark;
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Wow, I love this contribution. +0 −176 😄

@mkllnk mkllnk added the technical changes only These pull requests do not contain user facing changes and are grouped in release notes label Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
technical changes only These pull requests do not contain user facing changes and are grouped in release notes
Projects
Status: Test Ready 🧪
Development

Successfully merging this pull request may close these issues.

3 participants