Skip to content

Conversation

@albertcui
Copy link
Member

No description provided.

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');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the host be configurable?

const n = new Notification(
'Parsed $match'.replace('$match', payload.data.match_id),
{
body: 'Check out your performance.',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localize?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how. I was going to ask you. Strings are only within App, and this is an event listener.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you could mount this event listener within componentDidMount in App?

showLoginPrompt: true,
};

this.askForNotifiyPermission = this.askForNotifiyPermission.bind(this);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Notify

!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>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Get Tracked" may be poor phrasing with current concerns about data privacy

import constants from './components/constants';
// import registerServiceWorker from './registerServiceWorker';
import { unregister } from './registerServiceWorker';
import registerServiceWorker from './registerServiceWorker';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we previously had issues with the default CRA service worker. not sure if that's still an issue

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants