[WIP] Notifications for New Matches #1610
Conversation
Conflicts: package.json src/components/App/App.jsx src/components/Scenarios/ScenariosColumns.jsx
src/components/App/App.jsx
Outdated
| n.onclick = (event) => { | ||
| console.log(event); | ||
| event.preventDefault(); // prevent the browser from focusing the Notification's tab | ||
| window.open(`https://opendota.com/matches/${payload.data.match_id}`, '_blank'); |
There was a problem hiding this comment.
Should the host be configurable?
src/components/App/App.jsx
Outdated
| const n = new Notification( | ||
| 'Parsed $match'.replace('$match', payload.data.match_id), | ||
| { | ||
| body: 'Check out your performance.', |
There was a problem hiding this comment.
I don't know how. I was going to ask you. Strings are only within App, and this is an event listener.
There was a problem hiding this comment.
maybe you could mount this event listener within componentDidMount in App?
| showLoginPrompt: true, | ||
| }; | ||
|
|
||
| this.askForNotifiyPermission = this.askForNotifiyPermission.bind(this); |
src/components/App/App.jsx
Outdated
| !user && this.state.showLoginPrompt ? | ||
| <Prompt> | ||
| <img src="/assets/images/icons/icon-72x72.png" alt="logo" style={{ height: '25px', margin: '0 5px' }} /> | ||
| <h3 style={{ display: 'inline-block', margin: '0' }}>Get Tracked</h3> |
There was a problem hiding this comment.
"Get Tracked" may be poor phrasing with current concerns about data privacy
| @@ -11,8 +11,7 @@ import store from './store'; | |||
| import { getMetadata, getStrings, getAbilities, getNeutralAbilities, getAbilityIds } from './actions'; | |||
| import App from './components/App'; | |||
| import constants from './components/constants'; | |||
| // import registerServiceWorker from './registerServiceWorker'; | |||
| import { unregister } from './registerServiceWorker'; | |||
| import registerServiceWorker from './registerServiceWorker'; | |||
There was a problem hiding this comment.
we previously had issues with the default CRA service worker. not sure if that's still an issue
There was a problem hiding this comment.
nvm looks like you changed the path to another SW. I would probably make a copy of that file and change the path there so we can enable the CRA service worker at some later point
Conflicts: package.json
Conflicts: package-lock.json package.json
No description provided.