ChessCom / browser-extension

Customize your Chess.com experience
Other
48 stars 15 forks source link

Links open in new tab #85

Closed ManBearPigg closed 7 years ago

ManBearPigg commented 7 years ago

In app/components/Link.js, goTo(), changed chrome.tabs.query to chrome.tabs.create so that links to Chess.com open in new tab instead of hijacking the current tab without warning. The user might have work open :D

Passed run npm lint 👍

emilgoldsmith commented 7 years ago

Yeah looks like a good change to add now :).

Only weird things is that when you merged with master (which I guess should be fine) it looks like the commit history became quite weird? And also why is it CircleCI is not running? Any ideas on any of this @martynchamberlin ?

ManBearPigg commented 7 years ago

I was trying to update my fork when I guess I made an extra commit, but as far as I can tell the final state of my contribution just includes the changes we talked about. :D

emilgoldsmith commented 7 years ago

Yeah as you can see here it's all good :).

And I'm sure history should also not look so weird when we merge it in.

Biggest thing is that CircleCI isn't building for some reason.

martynchamberlin commented 7 years ago

@ManBearPigg Something has definitely gotten sideways on this PR:

Given that your changes are just a couple of lines in a single file, would you mind grabbing the latest and creating a fresh pull request?

emilgoldsmith commented 7 years ago

@ManBearPigg for the Sam part you probably didn't do this

EDIT: just to clarify this comment. If you don't set your email to the same email you use on github, Github doesn't recognize it as being your commit

ManBearPigg commented 7 years ago

Created a fresh pull request like you recommended. Also did a git config to solve the username issue. Should be good to go. 👍 Sorry for the confusion.

emilgoldsmith commented 7 years ago

Awesome I'll close this PR and take a look at the other one right now.