-
Notifications
You must be signed in to change notification settings - Fork 974
Converts ImportBrowserDataPanel into redux component #9485
Converts ImportBrowserDataPanel into redux component #9485
Conversation
7a4ed0e
to
fe19bdb
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.
I manually test this PR with toggling the option off and it couldn't.
it shows
windowStore.js:548 Uncaught TypeError: action.selected.forEach is not a function
Let me try it again, it was working correctly for me when I was testing it |
@darkdh can you please try again |
fe19bdb
to
2243e9b
Compare
props.browserNames = importBrowserDataDetail.map((browser) => browser.get('name')) | ||
props.browserIndexes = importBrowserDataDetail.map((browser) => browser.get('index')) | ||
props.isSupportingHistory = currentBrowser.get('history', false) | ||
props.isSupportingFavourites = currentBrowser.get('favorites', false) |
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.
s/Favourites/Favorites/
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.
Done
const importBrowserDataSelected = currentWindow.get('importBrowserDataSelected', Immutable.Map()) | ||
const importBrowserDataDetail = currentWindow.get('importBrowserDataDetail', Immutable.Map()) | ||
const index = importBrowserDataSelected.get('index', '0') | ||
const currentBrowser = importBrowserDataDetail.get(index, Immutable.Map()) |
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.
currentSelectedBrowser
would be better
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.
Done
data.favorites = this.props.favorites | ||
data.history = this.props.history | ||
|
||
if (this.props.type != null) { |
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.
we can remove this check. type will always has value which comes from muon.
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.
Done
|
||
componentWillMount () { | ||
if (this.props.selectedIndex == null) { | ||
windowActions.setImportBrowserDataSelected(0) |
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.
nice cleanup
} | ||
|
||
componentWillMount () { | ||
if (this.props.selectedIndex == null) { |
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 think this check can be removed. Is there any case it will have value when componentWillMount
being called?
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 would leave it
Resolves brave#9442 Auditors: @bsclifton @bridiver Test Plan:
2243e9b
to
701c9b4
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.
++
Submitter Checklist:
git rebase -i
to squash commits (if needed).Resolves #9442
Auditors: @bsclifton @bridiver
Test Plan:
Reviewer Checklist:
Tests