This repository has been archived by the owner on Dec 11, 2019. It is now read-only.
Removes non-primitive types from TabPage #9794
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bsclifton
suggested changes
Jul 3, 2017
| }] | ||
|
|
||
| const tabPageMenu = Menu.buildFromTemplate(template) | ||
| tabPageMenu.popup(getCurrentWindow()) |
There was a problem hiding this comment.
Would be great to have a unit test that ensures when windowConstants.WINDOW_ON_TAB_PAGE_MENU happens that electron.remote.Menu.buildFromTemplate fires and that popup is called on the instance that it returns 😄
|
|
||
| const frames = frameStateUtil.getNonPinnedFrames(state) || Immutable.List() | ||
| const tabsPerPage = Number(getSetting(settings.TABS_PER_PAGE)) | ||
| const tabPageFrames = frames.slice(action.get('index') * tabsPerPage, (action.get('index') * tabsPerPage) + tabsPerPage) |
There was a problem hiding this comment.
Would be good to read action.get('index') into a variable and skip processing if:
- index is null /undefined
- index is out of bounds (negative value, over the max number of tab pages)
NejcZdovc
force-pushed
the
refactor/#9793-tabpage
branch
2 times, most recently
from
ce69e5f
to
28baeee
Compare
|
@bsclifton can we merged this one? |
bridiver
approved these changes
Jul 5, 2017
| @@ -69,6 +70,15 @@ class TabPage extends React.Component { | |||
| e.preventDefault() | |||
| } | |||
|
|
|||
| onContextMenu (e) { | |||
| e.stopPropagation() | |||
| windowActions.onTabPageMenu(this.props.index) | |||
There was a problem hiding this comment.
just a nit, but I think windowActions. onTabPageContextMenu would be better here
Resolves brave#9793 Auditors: @bsclifton @bridiver Test Plan:
NejcZdovc
force-pushed
the
refactor/#9793-tabpage
branch
from
28baeee
to
a117b9f
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Submitter Checklist:
git rebase -ito squash commits (if needed).Resolves #9793
Auditors: @bsclifton @bridiver
Test Plan:
Reviewer Checklist:
Tests