jazzicon icon indicating copy to clipboard operation
jazzicon copied to clipboard

2.1.0 not published to npm

Open onetrickwolf opened this issue 2 years ago • 6 comments

Hello was trying to resolve a yarn audit issue tied to this library. I see the fix was introduced in the latest commit but I don't think it was ever published to npm.

image

I am currently using this as a workaround in my package.json:

"@metamask/jazzicon": "github:metamask/jazzicon#d923914fda6a8795f74c2e66134f73cd72070667",

onetrickwolf avatar Aug 08 '22 23:08 onetrickwolf

I'm seeing the same problem. Can anyone publish, please? @jmrossy, @whymarrh

welldan97 avatar Dec 09 '22 20:12 welldan97

This library is easier to use. https://www.npmjs.com/package/@cfx-kit/wallet-avatar

John-Oldman-Wang avatar May 19 '23 05:05 John-Oldman-Wang

I tried to verify if generated colors after https://github.com/MetaMask/jazzicon/pull/6 are the same. They are not. I don't think 2.1.0 version should be published.

Here's a test of hueShift function with zero shift:

// @file test.js

// Since index.js does not export hueShift function just eval the code in current scope
const sourcecode = require('fs').readFileSync('./index.js', 'utf-8')
eval(sourcecode)

console.log('Preset colors\n' + colors.slice().join(','))

// Force random=0.5 => hueShift amount=0 => expect no change in colors
generator = { random: () => 0.5 }
console.log('0 hueShift colors\n' + hueShift(colors.slice(), generator).join(','))

run node test.js

  • Before #6: :heavy_exclamation_mark: 0 hueShift do not match preset colors. Original implementation of hueShift was wrong.

    Preset colors
    #01888C,#FC7500,#034F5D,#F73F01,#FC1960,#C7144C,#F3C100,#1598F2,#2465E1,#F19E02
    0 hueShift colors
    #01898E,#FA7500,#03505E,#F93F01,#FB1860,#C8144D,#F5C400,#189AF2,#2366E1,#F29E02
    
  • After #6: :heavy_exclamation_mark: 0 hueShift do not match preset colors. New implementation of hueShift is wrong. :x: 0 hueShift after #6 do not match 0 hueShift before #6. New implementation of hueShift is wrong differently!

    Preset colors
    #01888C,#FC7500,#034F5D,#F73F01,#FC1960,#C7144C,#F3C100,#1598F2,#2465E1,#F19E02
    0 hueShift colors
    #01878c,#fc7600,#034f5d,#f73e01,#fc1961,#c7144d,#f3c200,#159af2,#2466e1,#f19d02
    
  • (Before #6 but upgrade [email protected] or [email protected], and replace "hexString()" with "hex()")

    Preset colors
    #01888C,#FC7500,#034F5D,#F73F01,#FC1960,#C7144C,#F3C100,#1598F2,#2465E1,#F19E02
    0 hueShift colors
    #01888C,#FC7500,#034F5D,#F73F01,#FC1960,#C7144C,#F3C100,#1598F2,#2465E1,#F19E02
    

lukaw3d avatar Oct 07 '23 02:10 lukaw3d

Sorry for the delayed response @lukaw3d, I will look into this soon.

Also sorry everyone else for the delay in releasing 2.1.0. This library is set up different from our other libraries so it's a lot more painful to release it. Hopefully we can get it synced soon.

mcmire avatar Oct 13 '23 01:10 mcmire

Likewise, sorry if my PR #6 modified the colors slightly, that wasn't intentional. Thank you @lukaw3d for testing more closely and spotting the difference.

I do still think there's benefit to reducing the dependencies here. Would someone like to fix the hueshift method? If not, I may be able to find time to take another crack at this.

jmrossy avatar Oct 14 '23 05:10 jmrossy

Sorry for the delay in getting back to this issue. For the time being I have reverted the changes in #6 in a new PR, #24, so that we can create a new release and then re-implement that PR later.

mcmire avatar Jul 18 '24 22:07 mcmire