-
Notifications
You must be signed in to change notification settings - Fork 973
Conversation
PR blocked by #9773 |
bcda03f
to
ba0ca31
Compare
ba0ca31
to
84aee6e
Compare
|
5d0301d
to
0f509d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test is failing locally when run individually and as part of a suite. Also travis shows the test as failing.
{ | ||
const currentIndex = frameStateUtil.getIndexByTabId(state, action.tabId) | ||
if (currentIndex === -1) { | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it was like this before as well but this should be returning state here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
Ideally this whole thing would happen in the browser side instead but I know this is just a small fix to an existing bug so it's ok in general. But longer term we should refactor it. Test problem and returning state are blockers though. |
0f509d4
to
96d3a14
Compare
@bbondy Test fixed as well. Should be good to go now |
96d3a14
to
a47b252
Compare
when you have multiple tab pages Resolves brave#9789 Auditors: @bsclifton Test Plan:
a47b252
to
4342b8c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test plan works, automated tests pass, lgtm
Fixes close tabs to the left/right
Doesn't apply without redux to 0.18.x so we're moving this to 0.19.x |
when you have multiple tab pages
Submitter Checklist:
git rebase -i
to squash commits (if needed).Resolves #9789
Auditors: @bsclifton
Test Plan:
Reviewer Checklist:
Tests