{"action":"created","issue":{"url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9212","repository_url":"https://api.github.com/repos/Automattic/wp-calypso","labels_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9212/labels{/name}","comments_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9212/comments","events_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9212/events","html_url":"https://github.com/Automattic/wp-calypso/pull/9212","id":187969156,"number":9212,"title":"EditGravatar: Add uploaded image to state","user":{"login":"v18","id":11487924,"avatar_url":"https://avatars.githubusercontent.com/u/11487924?v=3","gravatar_id":"","url":"https://api.github.com/users/v18","html_url":"https://github.com/v18","followers_url":"https://api.github.com/users/v18/followers","following_url":"https://api.github.com/users/v18/following{/other_user}","gists_url":"https://api.github.com/users/v18/gists{/gist_id}","starred_url":"https://api.github.com/users/v18/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/v18/subscriptions","organizations_url":"https://api.github.com/users/v18/orgs","repos_url":"https://api.github.com/users/v18/repos","events_url":"https://api.github.com/users/v18/events{/privacy}","received_events_url":"https://api.github.com/users/v18/received_events","type":"User","site_admin":false},"labels":[{"id":285010028,"url":"https://api.github.com/repos/Automattic/wp-calypso/labels/Me","name":"Me","color":"006b75","default":false},{"id":285010030,"url":"https://api.github.com/repos/Automattic/wp-calypso/labels/[Status]%20Ready%20to%20Merge","name":"[Status] Ready to Merge","color":"d4fb79","default":false}],"state":"closed","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2016-11-08T11:34:45Z","updated_at":"2016-11-17T16:29:52Z","closed_at":"2016-11-08T20:32:06Z","pull_request":{"url":"https://api.github.com/repos/Automattic/wp-calypso/pulls/9212","html_url":"https://github.com/Automattic/wp-calypso/pull/9212","diff_url":"https://github.com/Automattic/wp-calypso/pull/9212.diff","patch_url":"https://github.com/Automattic/wp-calypso/pull/9212.patch"},"body":"This PR adds the local image to the state, so that it can be used later to display the newly-uploaded Gravatar immediately. The changes to `<Gravatar/>` will come in a subsequent PR.\r\n\r\n## How to test\r\n1. Run using `ENABLE_FEATURES=me/edit-gravatar make run`\r\n2. Get a bearer token using this script:\r\n    ```\r\n    curl -# -X POST --data-urlencode \"grant_type=password\" --data-urlencode \"client_id=appid\" --data-urlencode \"client_secret=appsecret\" --data-urlencode \"username=username\" --data-urlencode \"password=password\" https://public-api.wordpress.com/oauth2/token\r\n    ```\r\n    You can use Calypso's app ID & secret, found [here](https://github.com/Automattic/wp-calypso/blob/a1c610c300e040a2c101bb5af6a66f7cd72b52d6/config/production.json#L108-L109).\r\n    If you have 2FA enabled, you can add this extra parameter to the call: `--data-urlencode     \"wpcom_otp=one-time-password\"`, or you can create a new application password to use instead.\r\n3. Save the bearer token to localStorage using `localStorage.setItem('bearerToken', 'your_bearer_token');`\r\n4. Go to `/me` and upload a new Gravatar, see that it gets saved to the state\r\n5. Refresh the page. See that the image gets removed from the state.\r\n\r\n## Showing the new image: local image vs cache-busting URL\r\n\r\nThe problem: after the new Gravatar is uploaded, it does not immediately change in the browser since the GET request for the image has a 5 minute cache directive. In order to have a smooth user experience, I'd like to show the new Gravatar immediately everywhere in Calypso.\r\n\r\nWe can show the new Gravatar in Calypso in two ways:\r\n1. Use a cache-busting URL to get the browser to download the new image\r\n2. Show the local image the user chose\r\n\r\nI opted for the local image for these reasons:\r\n- **A smoother user experience**: the local image can be shown right away and will not require extra time for downloading the new image\r\n- **Better for slower networks**: it's better to reduce the number of network requests when we can. In this case, there's no reason to download the new image from Gravatar.com since we already have it locally\r\n\r\n## Limitations\r\n1. Persistence - right now the temporary image is not persisted. This means that it goes away on hard refresh and will not show up in new tabs. I am ok with this limitation for now, and it can be revisited when we have more data (user complaints, stats, etc). If we end up persisting the state, we'll also need to account for Chrome's behaviour ([ref](https://github.com/Automattic/wp-calypso/pull/8456#issuecomment-256493017)) for not re-downloading the Gravatar until refresh.\r\n2. Does not show up in other environments (new browser, different device, etc). This limitation would be present either way - either using cache-busing or the local image. If we cared about solving it, we could persist the image in the database, or change the Gravatar API.\r\n\r\nThere were two previous discussions on this: [initial discussion](https://github.com/Automattic/wp-calypso/pull/8456#discussion_r83319355), and [more discussion](https://github.com/Automattic/wp-calypso/pull/8456#discussion_r85481764)\r\n\r\ncc @gwwar "},"comment":{"url":"https://api.github.com/repos/Automattic/wp-calypso/issues/comments/261295489","html_url":"https://github.com/Automattic/wp-calypso/pull/9212#issuecomment-261295489","issue_url":"https://api.github.com/repos/Automattic/wp-calypso/issues/9212","id":261295489,"user":{"login":"dmsnell","id":5431237,"avatar_url":"https://avatars.githubusercontent.com/u/5431237?v=3","gravatar_id":"","url":"https://api.github.com/users/dmsnell","html_url":"https://github.com/dmsnell","followers_url":"https://api.github.com/users/dmsnell/followers","following_url":"https://api.github.com/users/dmsnell/following{/other_user}","gists_url":"https://api.github.com/users/dmsnell/gists{/gist_id}","starred_url":"https://api.github.com/users/dmsnell/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dmsnell/subscriptions","organizations_url":"https://api.github.com/users/dmsnell/orgs","repos_url":"https://api.github.com/users/dmsnell/repos","events_url":"https://api.github.com/users/dmsnell/events{/privacy}","received_events_url":"https://api.github.com/users/dmsnell/received_events","type":"User","site_admin":false},"created_at":"2016-11-17T16:29:52Z","updated_at":"2016-11-17T16:29:52Z","body":"It seems to me like instead of creating an altogether new temporary image API, combined with the fact that none of this persists, we could just set the \"temporary\" image as the actual gravatar avatar URL in the application state. it doesn't matter if we're getting the image from Gravatar or from local cache; it only matters that the image shows the right picture.\r\n\r\nif we stored the temporary URL as the avatar URL as if it were actually the case then we could get rid of all the functionality and nomenclature built up for the temp images. this could dramatically simplify a bunch of the `if ( tempURL )` and `tempURL || otherURL` snippets"}}
{"actor":{"display_login":"dmsnell"}}