diff --git a/src/images/issue-bg.png b/src/images/issue-bg.png new file mode 100644 index 0000000..d767c8c Binary files /dev/null and b/src/images/issue-bg.png differ diff --git a/src/images/issue.png b/src/images/issue.png new file mode 100644 index 0000000..d370ee5 Binary files /dev/null and b/src/images/issue.png differ diff --git a/src/images/pr-bg.png b/src/images/pr-bg.png new file mode 100644 index 0000000..662d9f0 Binary files /dev/null and b/src/images/pr-bg.png differ diff --git a/src/images/pr.png b/src/images/pr.png new file mode 100644 index 0000000..4211c33 Binary files /dev/null and b/src/images/pr.png differ diff --git a/src/pages/Notifications/Scene.js b/src/pages/Notifications/Scene.js index 677c536..b84d0a9 100644 --- a/src/pages/Notifications/Scene.js +++ b/src/pages/Notifications/Scene.js @@ -27,7 +27,7 @@ function stringOfType (type) { } } -function getMessageFromReasons (reasons, type) { +export function getMessageFromReasons (reasons, type) { switch (reasons[reasons.length - 1].reason) { case Reasons.ASSIGN: return 'You were assigned this ' + stringOfType(type); @@ -516,9 +516,9 @@ export default function Scene ({ ? Icon.NotificationsOn : Icon.NotificationsOff; - console.log('notifications', notifications) - console.log('isFirstTimeUser', isFirstTimeUser) - console.log('notificationsPermission', notificationsPermission) + // console.log('notifications', notifications) + // console.log('isFirstTimeUser', isFirstTimeUser) + // console.log('notificationsPermission', notificationsPermission) if (isFirstTimeUser && notifications.length > 5) { // alert('hello, clear ur shit'); @@ -722,7 +722,13 @@ export default function Scene ({ }) : undefined} /> - + { diff --git a/src/pages/Notifications/index.js b/src/pages/Notifications/index.js index cf68d01..7001573 100644 --- a/src/pages/Notifications/index.js +++ b/src/pages/Notifications/index.js @@ -11,7 +11,9 @@ import { routes } from '../../constants'; import { Filters } from '../../constants/filters'; import { Status } from '../../constants/status'; import { Reasons, Badges } from '../../constants/reasons'; -import Scene from './Scene'; +import Scene, { getMessageFromReasons } from './Scene'; +import issueIcon from '../../images/issue-bg.png'; +import prIcon from '../../images/pr-bg.png'; const PER_PAGE = 10; @@ -113,9 +115,15 @@ const decorateWithScore = notification => ({ }); class NotificationsPage extends React.Component { + constructor (props) { + super(props); + + this.notificationSent = false; + } + state = { currentTime: moment(), - prevNotifications: [], + notificationSent: false, isFirstTimeUser: false, isSearching: false, query: null, @@ -139,6 +147,15 @@ class NotificationsPage extends React.Component { }, 8 * 1000); } + shouldComponentUpdate (nextProps, nextState) { + if (this.props.notificationsApi.newChanges !== nextProps.notificationsApi.newChanges) { + this.notificationSent = false; + } + // The idea here is if we've just updated the prevNotifications state, then + // we don't want to trigger a rerender. + return nextState.prevNotifications === this.state.prevNotifications; + } + componentWillUnmount () { clearInterval(this.syncer); } @@ -200,31 +217,44 @@ class NotificationsPage extends React.Component { this.props.notificationsApi.setNotificationsPermission(...args); } - sendWebNotification = () => { - var img = '../images/icon.png'; - var text = 'HEY! Your task "null" is now overdue.'; - var notification = new Notification('Meteorite', { body: text, icon: img }); - } - - render () { - if (!this.props.authApi.token) { - return + sendWebNotification = newNotifcations => { + if (this.notificationSent || newNotifcations.length === 0) { + return; } - const { - fetchNotifications, - markAsRead, - markAllAsStaged, - clearCache, - notificationsPermission, - notifications, - loading: isFetchingNotifications, - error: fetchingNotificationsError, - } = this.props.notificationsApi; + // Set this even if we don't actually send the notification due to permissions. + this.notificationSent = true; + + // No permission, no notification. + if (this.props.notificationsApi.notificationsPermission !== 'granted') { + return; + } + + const n = newNotifcations[0]; + const reasonByline = getMessageFromReasons(n.reasons, n.type); + + const additionalInfo = newNotifcations.length > 1 + ? ` (+${newNotifcations.length} more)` + : ''; + + const notification = new Notification(n.name + additionalInfo, { + body: reasonByline, + icon: n.type === "Issue" ? issueIcon : prIcon, + badge: n.type === "Issue" ? issueIcon : prIcon, + }); + + notification.addEventListener('click', () => { + this.enhancedOnStageThread(n.id, n.repository); + window.open(n.url); + }) + + // Manually close for legacy browser support. + setTimeout(notification.close.bind(notification), 4000); + } + + getFilteredNotifications = () => { + const {notifications} = this.props.notificationsApi; - // @TODO Move all this out of the render method. - // nick, do this ^ so you can fire off a web noti when the filtered/final - // notifications have a diff let filterMethod = () => true; switch (this.state.activeFilter) { case Filters.PARTICIPATING: @@ -287,6 +317,45 @@ class NotificationsPage extends React.Component { ) } + if (this.props.notificationsApi.newChanges) { + // we shouldn't do it like this. instead, we should have an additional state called + // "new changes" or something that the notifications api knows about. + // this will be whatever we get in the syncing/fetching response + console.log('send', this.props.notificationsApi.newChanges) + this.sendWebNotification(this.props.notificationsApi.newChanges); + } + + console.warn(this.props.notificationsApi); + + return { + notifications: scoredAndSortedNotifications, + queuedCount: notificationsQueued.length, + stagedCount: notificationsStaged.length, + closedCount: notificationsClosed.length, + }; + } + + render () { + if (!this.props.authApi.token) { + return + } + + const { + fetchNotifications, + markAsRead, + markAllAsStaged, + clearCache, + notificationsPermission, + loading: isFetchingNotifications, + error: fetchingNotificationsError, + } = this.props.notificationsApi; + const { + notifications: scoredAndSortedNotifications, + queuedCount, + stagedCount, + closedCount, + } = this.getFilteredNotifications(); + let firstIndex = (this.state.currentPage - 1) * PER_PAGE; let lastIndex = (this.state.currentPage * PER_PAGE); let notificationsOnPage = scoredAndSortedNotifications.slice(firstIndex, lastIndex); @@ -311,9 +380,9 @@ class NotificationsPage extends React.Component { isFirstTimeUser={this.state.isFirstTimeUser} setNotificationsPermission={this.setNotificationsPermission} notificationsPermission={notificationsPermission} - queuedCount={notificationsQueued.length} - stagedCount={notificationsStaged.length} - closedCount={notificationsClosed.length} + queuedCount={queuedCount} + stagedCount={stagedCount} + closedCount={closedCount} stagedTodayCount={stagedTodayCount || 0} first={firstNumbered} last={lastNumbered} diff --git a/src/providers/Notifications.js b/src/providers/Notifications.js index f0aaf46..424215e 100644 --- a/src/providers/Notifications.js +++ b/src/providers/Notifications.js @@ -4,6 +4,7 @@ import {StorageProvider, LOCAL_STORAGE_PREFIX} from './Storage'; import {Status} from '../constants/status'; const BASE_GITHUB_API_URL = 'https://api.github.com'; +const PER_PAGE = 50; function cleanResponseUrl (url) { return url @@ -62,12 +63,17 @@ class NotificationsProvider extends React.Component { syncing: false, loading: false, error: null, + newChanges: null, notificationsPermission: this.props.getUserItem('notificationsPermission') || 'default', } shouldComponentUpdate (nextProps, nextState) { + // Don't try to rerender if we're just setting the new changes. + if (this.state.newChanges !== nextState.newChanges) { + return false; + } // Update if our state changes. if ((this.state.loading !== nextState.loading) || (this.state.error !== nextState.error)) { @@ -102,7 +108,7 @@ class NotificationsProvider extends React.Component { headers['If-Modified-Since'] = this.last_modified; } - return fetch(`${BASE_GITHUB_API_URL}/notifications?page=${page}`, { + return fetch(`${BASE_GITHUB_API_URL}/notifications?page=${page}&per_page=${PER_PAGE}`, { method: 'GET', headers: headers }) @@ -129,7 +135,11 @@ class NotificationsProvider extends React.Component { this.setState({syncing: true}); return this.requestPage(page, optimizePolling) .then(({headers, json}) => { - if (json === null) return []; + // This means that we got a response where nothing changed. + if (json === null) { + this.setState({newChanges: null}); + return []; + } let nextPage = null; const links = headers['link']; if (links && links.next && links.next.page) { @@ -148,7 +158,7 @@ class NotificationsProvider extends React.Component { if (this.state.loading) { // Don't try to fetch if we're already fetching - return Promise.reject(); + return; } this.setState({ loading: true }); @@ -167,26 +177,33 @@ class NotificationsProvider extends React.Component { // Apparently this means that a user has no notifications (makes sense). // So I guess we should purge our cache? This brings up the great point // of us having stale cache. How can we detect that a notifcation was seen? + // Update ^ we can't so we'll provide the tools for users to easily handle this. } - notificationsChunk.forEach(n => { + const processedNotifications = notificationsChunk.map(n => { const cached_n = this.props.getItemFromStorage(n.id); // If we've seen this notification before. if (cached_n) { // Something's changed, we want to push if (cached_n.updated_at !== n.updated_at) { - this.updateNotification(n, cached_n.reasons); - return; + return this.updateNotification(n, cached_n.reasons);; } // This means that something didn't update, which means the page we're // currently processing has stale data so we don't need to fetch the next page. everythingUpdated = false; + return cached_n; } else { // Else, update the cache. - this.updateNotification(n); + return this.updateNotification(n); } }); + if (this.last_modified && notificationsChunk.length < PER_PAGE) { + // `this.last_modified` being set means this isn't our first fetch. + // We don't want to send notifications for the first time the page loads. + this.setState({newChanges: processedNotifications}); + } + if (nextPage && everythingUpdated) { // Still need to fetch more updates. this.fetchNotifications(nextPage, false); @@ -228,7 +245,7 @@ class NotificationsProvider extends React.Component { if (this.state.loading) { // Don't try to fetch if we're already fetching - return Promise.reject(); + return; } this.setState({ loading: true }); @@ -346,6 +363,14 @@ class NotificationsProvider extends React.Component { reasons = [newReason]; } + const commentNumber = n.subject.latest_comment_url + ? n.subject.latest_comment_url.split('/').pop() + : null; + + const url = commentNumber + ? cleanResponseUrl(n.subject.url) + '#issuecomment-' + commentNumber + : cleanResponseUrl(n.subject.url); + // Notification model const value = { id: n.id, @@ -355,12 +380,13 @@ class NotificationsProvider extends React.Component { reasons: reasons, type: n.subject.type, name: n.subject.title, - url: cleanResponseUrl(n.subject.url), + url: url, repository: n.repository.full_name, number: n.subject.url.split('/').pop(), repositoryUrl: cleanResponseUrl(n.repository.url) }; this.props.setItemInStorage(n.id, value); + return value; } render () {