Update ada.svg - Githubissues

activemerchant / payment_icons

An easy to use library that allows you to manage and access payment icons

MIT License

144 stars 422 forks source link

Update ada.svg #1137

Closed ptrdsh closed 6 months ago

ptrdsh commented 7 months ago

Why are you adding this icons?

I'm updating this icon because the previous version was not very well identifiable ..

Help us identify yourself

[x] I'm working/collaborating with the brand directly and they have provided the icons.
[ ] I'm associated with the brand and I've read all the brand icon’s guidelines.
[ ] I'm an individual and I've read all the brand icon’s guidelines.

Link to the brand guidelines: https://cardanofoundation.org/en/trademark-policy

Attach a screenshot of the icon along side the example Visa icon

adeniyiao commented 7 months ago

Hi @ptrdsh ,

Kindly address the issues with the checks.

Regards

ptrdsh commented 7 months ago

Thanks @adeniyiao and sorry for the oversight! added now.

adeniyiao commented 7 months ago

Thanks @adeniyiao and sorry for the oversight! added now.

No worries, could you please fix this new issue. Thanks

{:message=>"The 'ada' SVG file should have a <title> tag as first child of the root <svg> tag"}.
Expected: "title"
  Actual: "g"

ptrdsh commented 7 months ago

I hope its ok now @adeniyiao - thanks for your patience and effort :)

ptrdsh commented 6 months ago

@adeniyiao - its been an odd ride of back and forth, for which im terribly sorry, but could you look into this once more? 🙏

ptrdsh commented 6 months ago

@hellicarusprime - first off.. thanks a lot and sorry for the back and forth!

I please kindly need help on this error - i tried resolving it, but as evident above, it didnt work.. I now plainly copy-pasted the attributes from the svg that worked previously and compared them to other new svg change PRs (like the recent AmEx change), but to no avail either.. by now im clueless what the problem is.

Thanks for your help!

Failure:
PaymentIconTest#test_Every_payment_SVG_meets_accessibility_requirements [test/unit/payment_icon_test.rb:89]:
{:message=>"The 'a' ID should be pi-ada-a (missing 'pi-' prefix)"}.
Expected /pi-(.*)/ to match "a".

-> to me, it appears as if the id has the required "pi-" prefix in there...

<svg xmlns="http://www.w3.org/2000/svg" role="img" viewBox="0 0 38 24" width="38" height="24" aria-labelledby="pi-ada"><title id="pi-ada">Cardano</title>

hellicarusprime commented 6 months ago

@ptrdsh It might be because this <clipPath id="a"><path fill="#fff" d="M0 0h38v24H0z"></path></clipPath> is missing the pi- for the id. It should probably be <clipPath id="pi-a">...

ptrdsh commented 6 months ago

AH! there is another id reference in the end 🤦 i hope this is it.

Thank you @hellicarusprime !!