Skip to content
This repository has been archived by the owner on Jul 15, 2022. It is now read-only.

Add Jarvis Network icons #1489

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Jarvis Network icons #1489

wants to merge 1 commit into from

Conversation

0xyPhilic
Copy link

Context (issues, jira)

Description / Usage

Expectations

  • Test coverage: The changes of this PR are covered by test. Unit test were added with mocks when depending on a backend/device.
  • No impact: The changes of this PR have ZERO impact on the userland. Meaning, we can use these changes without modifying LLD/LLM at all. It will be a "noop" and the maintainers will be able to bump it without changing anything.

@0xyPhilic 0xyPhilic requested a review from a team October 28, 2021 17:09
@vercel
Copy link

vercel bot commented Oct 28, 2021 •
edited
Loading

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/ledgerhq/ledger-live-common/2PQG6fDVUBcWyETTrTNVf5exeAVr
✅ Preview: https://ledger-live-common-git-fork-jarvis-network-jarv-e43dcc-ledgerhq.vercel.app

@gre gre added the coin-icon label Oct 29, 2021
gre
gre suggested changes Dec 7, 2021
View reviewed changes
src/data/icons/svg/jCHF.svg
<circle id="Ellipse_73" data-name="Ellipse 73" cx="72.368" cy="72.368" r="72.368" transform="translate(0 0)" fill="#fff"/>
<path id="Ellipse_73_-_Outline" data-name="Ellipse 73 - Outline" d="M72.5,6.042a66.458,66.458,0,1,0,46.993,19.465A66.458,66.458,0,0,0,72.5,6.042M72.5,0A72.5,72.5,0,1,1,0,72.5,72.5,72.5,0,0,1,72.5,0Z" transform="translate(0 0)" fill="#d80027"/>
<circle id="Ellipse_78" data-name="Ellipse 78" cx="60.214" cy="60.214" r="60.214" transform="translate(12.153 12.153)" fill="#d80027"/>
<g id="Group_21" data-name="Group 21" transform="translate(29.284 29.282)">
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the contribution. could you merge master in? we have extra test that do some checks to ensure icons are fine. Thanks

Unfortunately, we can't afford to have id=, class= or <style> in these SVG because we will convert them back into react class.

Copy link
Author

Choose a reason for hiding this comment

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

Hi @gre , do you want me to merge the branch in master and reopen the PR from our master as I didn't fully understand?

I'll see with our designer to address the issues mentioned with id=, class= and <style> and will make a commit with the new files :)

@0xyPhilic 0xyPhilic requested a review from a team as a code owner April 11, 2022 18:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gre gre gre requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
coin-icon
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xyPhilic @gre