Enable favorite shots on My Shots #4984
Conversation
| @@ -315,6 +315,7 @@ shotIndexFavoriteIcon = | |||
| .title = This is a favorite shot and it does not expire | |||
| shotIndexSyncedShot = | |||
| .title = Shot taken on another device | |||
| shotIndexAlertErrorFavoriteShot = Error updating favorite shot status | |||
There was a problem hiding this comment.
Not sure about this message. Perhaps @johngruen can advise. It's similar to the one on the shot page. (That one probably should be updated?) It's used for favorite and un-favorite actions.
chenba
force-pushed
the
4934-fav-myshots
branch
from
5b02440 to
5f83d19
Compare
server/src/pages/shotindex/view.js
Outdated
|
|
||
| onClickFavorite(shot) { | ||
| controller.toggleFavoriteShot(shot); | ||
| sendEvent("favorite", "myshots-tile"); |
There was a problem hiding this comment.
METRICS.md should be updated with this event
| if (!resp.ok) { | ||
| window.Raven.captureException( | ||
| new Error(`Error calling /api/set-expiration: ${resp.status} ${resp.statusText}`)); | ||
| throw new Error(); |
There was a problem hiding this comment.
Usually I do: let exc = new Error(...); window.Raven.captureException(exc); throw exc;
server/src/pages/shotindex/view.js
Outdated
| @@ -148,6 +148,9 @@ class Body extends React.Component { | |||
| renderErrorMessages() { | |||
| return ( | |||
| <div> | |||
| <Localized id=""> | |||
There was a problem hiding this comment.
Why do the other Localized's have ids and this one doesn't?
There was a problem hiding this comment.
D'oh! I deleted the id of some pasted code but didn't insert the new one.
| } catch (err) { | ||
| // The error could be one from the server or a network level error from | ||
| // fetch; we display a generic message either way as it's (probably?) | ||
| // friendlier than percolating a fetch error message up to the user. |
There was a problem hiding this comment.
Probably good to do console.warn("Error updating expiration:", err); so it isn't entirely swallowed
chenba
force-pushed
the
4934-fav-myshots
branch
from
5f83d19 to
cd888a3
Compare
chenba
force-pushed
the
4934-fav-myshots
branch
2 times, most recently
from
166466d to
57e7a3e
Compare
chenba
force-pushed
the
4934-fav-myshots
branch
from
57e7a3e to
7ff900d
Compare
| @@ -328,6 +328,8 @@ These are events that an add-on user can encounter on a shot they own | |||
| 6. [x] Right-click (or get the context menu) anywhere on the page `contextmenu/background`, `contextmenu/shot-tile`, `contextmenu/search`, or `contextmenu/header` depending on where the user clicks. | |||
| 7. [x] Click download from tile: `web/download/myshots-tile` | |||
| 8. [x] Clear search with button: `web/clear-search/button` | |||
| 8. [x] Click to favorite: `web/favorite/myshots-tile` | |||
| 8. [x] Click to un-favorite: `web/unfavorite/myshots-tile` | |||
There was a problem hiding this comment.
This is new since the previous review. Now it track seting and unsetting a shot as a favorite with separate events.
| @@ -463,6 +468,16 @@ class Card extends React.Component { | |||
| this.downloadButton.blur(); | |||
| sendEvent("download", "myshots-tile"); | |||
| } | |||
|
|
|||
| onClickFavorite(shot) { | |||
| if (shot.isFavorite) { | |||
There was a problem hiding this comment.
isFavorite is set on each shot after it's instantiated from the JSON data and updated in toggleFavoriteShot. Could consider changing it to a function in shot that returns a bool based on expiration, similar to what was done for isOwner.
Fixes #4934