-
-
Notifications
You must be signed in to change notification settings - Fork 725
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
base: master
Are you sure you want to change the base?
Remove unused renderSvg, ofnDisableScroll, integer, ofnScrollTo angular directives #12979
Conversation
There was a problem hiding this 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 🏅
svg { | ||
path { | ||
fill: $disabled-dark; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although this rule isn't scoped to render-svg, it probably was intended to be. It looks to me like there's never an svg
in this context:
https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/views/shops/_fat.html.haml#L13-L14
https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/views/producers/_fat.html.haml#L25-L26
There was a problem hiding this 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
😄
What? Why?
This partially addresses #9477 and removes four unused angular directives.
renderSvg
directive was introduced in bc93ce5 and seems to have been removed in Remove taxon icons #5713ofnDisableScroll
directive was introduced in 22aaa24 and seems to have been removed in 59c13b9integer
directive was introduced in 0895bd8 and seems to have been removed in 01c4882ofnScrollTo
directive was introduced in de369d9 and seems to have been removed in 427f535What should we test?
Confirm the directives aren't called anywhere e.g.
And on staging:
Release notes