Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix activeTabUrl #2005

Merged
merged 6 commits into from
Nov 19, 2020
Merged

Fix activeTabUrl #2005

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
548b239
Fix transaction header title
rickycodes Nov 19, 2020
342b3f8
Get activeTabUrl from props now
rickycodes Nov 19, 2020
fa2c5c6
Update tests
rickycodes Nov 19, 2020
c75f713
Update CHANGELOG
rickycodes Nov 19, 2020
b346a1e
bump
rickycodes Nov 19, 2020
9052163
fixwcicon
estebanmino Nov 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions app/components/UI/ApproveTransactionReview/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,14 @@ class ApproveTransactionReview extends PureComponent {
* Error coming from gas component
*/
gasError: PropTypes.string,
primaryCurrency: PropTypes.string
/**
* Primary currency, either ETH or Fiat
*/
primaryCurrency: PropTypes.string,
/**
* Active tab URL, the currently active tab url
*/
activeTabUrl: PropTypes.string
};

state = {
Expand Down Expand Up @@ -534,7 +541,6 @@ class ApproveTransactionReview extends PureComponent {

render = () => {
const {
activeTabUrl,
host,
tokenSymbol,
viewDetails,
Expand All @@ -549,6 +555,7 @@ class ApproveTransactionReview extends PureComponent {
primaryCurrency,
currentCurrency,
gasError,
activeTabUrl,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we were still trying to get it from component state instead of props

transaction: { origin }
} = this.props;

Expand Down
2 changes: 1 addition & 1 deletion app/components/UI/TransactionHeader/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ const TransactionHeader = props => {
let title = '';
if (originIsDeeplink) title = renderShortAddress(spenderAddress);
else if (originIsWalletConnect) title = getHost(origin.split(WALLET_CONNECT_ORIGIN)[1]);
else title = getHost(currentEnsName || url);
else title = getHost(currentEnsName || url || origin);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here we fall back to origin, but it's not really necessary (better than potentially showing undefined if this ever breaks again)


return <Text style={styles.domainUrl}>{title}</Text>;
};
Expand Down
9 changes: 2 additions & 7 deletions app/util/transactions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,10 +414,5 @@ export function getNormalizedTxState(state) {
return { ...state.transaction, ...state.transaction.transaction };
}

export function getActiveTabUrl({ browser = {} }) {
let tab;
if (browser.tabs && browser.activeTab) {
tab = browser.tabs.find(({ id }) => id === browser.activeTab);
}
return tab ? tab.url : '';
}
export const getActiveTabUrl = ({ browser = {} }) =>
browser.tabs && browser.activeTab && browser.tabs.find(({ id }) => id === browser.activeTab)?.url;