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

2.1.0 not published to npm #10

Open
onetrickwolf opened this issue Aug 8, 2022 · 6 comments
Open

2.1.0 not published to npm #10

onetrickwolf opened this issue Aug 8, 2022 · 6 comments

Comments

@onetrickwolf
Copy link

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",
@welldan97
Copy link

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

@John-Oldman-Wang

This comment was marked as off-topic.

@lukaw3d
Copy link

lukaw3d commented Oct 7, 2023

I tried to verify if generated colors after #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:
    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:
    0 hueShift do not match preset colors. New implementation of hueShift is wrong.
    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
    

@mcmire
Copy link

mcmire commented Oct 13, 2023

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.

@jmrossy
Copy link

jmrossy commented Oct 14, 2023

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.

@mcmire
Copy link

mcmire commented Jul 18, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants