KyleAMathews / react-gravatar

React component for rendering a gravatar profile image
http://kyleamathews.github.io/react-gravatar/
MIT License
279 stars 33 forks source link

Implement Recommended Gravatar Format? #86

Closed AlexanderNull closed 8 years ago

AlexanderNull commented 8 years ago

The recommended input format for an email should be in lowercase and trimmed of any white space in order to get the correct MD5 hash match. It doesn't look like this component implements such formatting on its own. Can we add this formatting into the component automatically or alternatively update the readme so that users know what format their input should be in?

KyleAMathews commented 8 years ago

This could be added to the component. Would you like to create a PR? On Tue, Aug 2, 2016 at 8:25 PM Alexander notifications@github.com wrote:

The recommended input format for an email should be in lowercase and trimmed of any white space in order to get the correct MD5 hash match. It doesn't look like this component implements such formatting on its own. Can we add this formatting into the component automatically or alternatively update the readme so that users know what format their input should be in?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/KyleAMathews/react-gravatar/issues/86, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEVh87sTMEprnr3LHQLoGlt5l9ti06Fks5qb_wCgaJpZM4JbPS5 .

AlexanderNull commented 8 years ago

Certainly, I should be able to tackle it this weekend. Is there a significant use case for allowing case sensitive email input? If so we can maintain both functionality side by side with an additional prop.

KyleAMathews commented 8 years ago

I can't think of one. Emails aren't case sensitive (right?). On Tue, Aug 2, 2016 at 8:56 PM Alexander notifications@github.com wrote:

Certainly, I should be able to tackle it this weekend. Is there a significant use case for allowing case sensitive email input? If so we can maintain both functionality side by side with an additional prop.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/KyleAMathews/react-gravatar/issues/86#issuecomment-237120644, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEVh3oyvVeYZhsbwwXHb4sh5hIuXMi0ks5qcANRgaJpZM4JbPS5 .

AlexanderNull commented 8 years ago

Hmm, apparently in the SMTP spec they can be (http://www.ietf.org/rfc/rfc2821.txt) for the local address portion, although it's discouraged in the spec from making use of case sensitivity. Hadn't anticipated that.

KyleAMathews commented 8 years ago

A better question perhaps is if the Gravatar system always lowercases emails when someone sets up a picture. If that's the case then we'd have no reason not to do the same. On Tue, Aug 2, 2016 at 9:08 PM Alexander notifications@github.com wrote:

Hmm, apparently in the SMTP spec they can be ( http://www.ietf.org/rfc/rfc2821.txt) for the local address portion, although it's discouraged in the spec from making use of case sensitivity. Hadn't anticipated that.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/KyleAMathews/react-gravatar/issues/86#issuecomment-237122377, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEVh1k2c6X6gOQ22stlzflvpcctkUSaks5qcAYpgaJpZM4JbPS5 .

AlexanderNull commented 8 years ago

I'll take a look.

AlexanderNull commented 8 years ago

Looks like you can't register two emails with the same characters, different case. From what I can find online, they changed to this case insensitive format a few years ago. Added a PR to both trim and lowercase the email before making hash as recommended by Gravatar. https://github.com/KyleAMathews/react-gravatar/pull/93

KyleAMathews commented 8 years ago

Fixed in #93

satazor commented 8 years ago

@KyleAMathews can you release a new version? Thanks.

KyleAMathews commented 8 years ago

@satazor 2.5.1 is out

satazor commented 8 years ago

Ty @KyleAMathews. Also take a look at #130 when you got some time.