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

Add a fallback.svg for dynamic fallbacks #115

Closed
wants to merge 8 commits into from

Conversation

danielbachhuber
Copy link

@danielbachhuber danielbachhuber commented Nov 20, 2020

@vercel
Copy link

vercel bot commented Nov 20, 2020

@danielbachhuber is attempting to deploy a commit to a Personal Account owned by @Kikobeats on Vercel.

@Kikobeats first needs to authorize it.

@Kikobeats
Copy link
Member

wow thanks for that, it looks pretty sick!

Your current implementation is using it as a provider. I think I like that idea and we can rename it into "identicon" since the fallback query parameter is already implemented.

what do you think about generating the color in a pseudo-random way, so the user doesn't need to think about start and end?

For example, you can generate those value based on the text input

@danielbachhuber
Copy link
Author

Your current implementation is using it as a provider. I think I like that idea and we can rename it into "identicon" since the fallback query parameter is already implemented.

Ah, yeah, that'd be a better approach. To confirm, rename my fallback.svg to identicon.svg, and keep the same query params for controlling it?

what do you think about generating the color in a pseudo-random way, so the user doesn't need to think about start and end?

Do you have an algorithm you'd suggest? I'd like to keep the parameters as options so I can control the color in my app.

Oh, another thought: I defaulted to a solid color because I thought that might be nicer than a random gradient.

@Kikobeats
Copy link
Member

We can implement this solution

https://github.com/tobiaslins/avatar/blob/master/src/image.js#L9

What do you think:

  • generate a linear gradient using text by default
  • be possible setup linear gradient using query params (start and end)

In that way, we can have a good first impression, but also powerful to customize in case you want 🙂

A variation about the default color be to use just a plain color: https://github.com/Gustu/string-to-color

@danielbachhuber
Copy link
Author

I'm amenable to that approach.

Where would you like the functions stored, and do you have any other code structure suggestions before I finish this up?

@Kikobeats
Copy link
Member

just renaming it as identicon provider should be enough; all the code can live there 🙂

@vercel
Copy link

vercel bot commented Dec 14, 2020

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/kikobeats/unavatar/24PJ8nrfPzQEfV1ZB4Gn6QaEYufX
✅ Preview: https://unavatar-git-fork-danielbachhuber-master-kikobeats.vercel.app

@danielbachhuber
Copy link
Author

@Kikobeats Sorry, meant to comment too: this PR is ready.

@Kikobeats
Copy link
Member

Thanks, let me review it quickly 🙂

@danielbachhuber
Copy link
Author

@Kikobeats Still thinking about landing this?

@Kikobeats
Copy link
Member

Kikobeats commented Jul 12, 2021

hmm I'm testing this locally and looks there is a problem related to color transformation

TypeError: Cannot read property '2' of undefined
    at generateGradient (/Users/kikobeats/Projects/unavatar/src/providers/identicon.js:60:43)
    at module.exports (/Users/kikobeats/Projects/unavatar/src/providers/identicon.js:79:40)
    at /Users/kikobeats/Projects/unavatar/src/index.js:44:12
    at Layer.handle [as handle_request] (/Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/route.js:137:13)
    at Route.dispatch (/Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/route.js:112:3)
    at Layer.handle [as handle_request] (/Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/layer.js:95:5)
    at /Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/index.js:281:22
    at Function.process_params (/Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/kikobeats/Projects/unavatar/node_modules/express/lib/router/index.js:275:10)

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

Successfully merging this pull request may close these issues.

2 participants