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

chore: active url to save with useState #12815

Open
wants to merge 2 commits into
base: fix/browser-tab-convert-ts
Choose a base branch
from
+57 −52
Open

chore: active url to save with useState #12815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c7fc7fc
active url to save with useState
tommasini Dec 20, 2024
ea27465
update notifyAllConnection dependency
tommasini Dec 20, 2024
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
7 changes: 2 additions & 5 deletions app/components/UI/BrowserUrlBar/BrowserUrlBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import Icon, {
import { BrowserUrlBarProps } from './BrowserUrlBar.types';
import stylesheet from './BrowserUrlBar.styles';
import { BrowserViewSelectorsIDs } from '../../../../e2e/selectors/Browser/BrowserView.selectors';
import Url from 'url-parse';
import { regex } from '../../../../app/util/regex';
import { strings } from '../../../../locales/i18n';
import { BrowserURLBarSelectorsIDs } from '../../../../e2e/selectors/Browser/BrowserURLBar.selectors';
Expand All @@ -44,7 +43,7 @@ const BrowserUrlBar = forwardRef<TextInput, BrowserUrlBarProps>(
onBlur,
onChangeText,
connectedAccounts,
activeUrlRef,
activeUrl,
},
ref,
) => {
Expand Down Expand Up @@ -147,9 +146,7 @@ const BrowserUrlBar = forwardRef<TextInput, BrowserUrlBarProps>(
hostInfo: {
metadata: {
// TODO: This is not an origin, it's a hostname
origin:
activeUrlRef.current &&
new URLParse(activeUrlRef.current).hostname,
origin: activeUrl && new URLParse(activeUrl).hostname,
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion app/components/UI/BrowserUrlBar/BrowserUrlBar.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ export type BrowserUrlBarProps = {
onBlur: () => void;
onChangeText: (text: string) => void;
connectedAccounts: string[];
activeUrlRef: React.RefObject<string>;
activeUrl: string;
};
78 changes: 42 additions & 36 deletions app/components/Views/BrowserTab/BrowserTab.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
const urlBarResultsRef = useRef<UrlAutocompleteRef>(null);
const [isSecureConnection, setIsSecureConnection] = useState(false);

const activeUrl = useRef('');
// const activeUrl = useRef('');
const [activeUrl, setActiveUrl] = useState('');
const title = useRef<string>('');
const icon = useRef<ImageSourcePropType | undefined>();
const backgroundBridges = useRef<
Expand All @@ -154,15 +155,15 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
const wizardScrollAdjusted = useRef(false);
const permittedAccountsList = useSelector((state: RootState) => {
const permissionsControllerState = selectPermissionControllerState(state);
const hostname = new URLParse(activeUrl.current).hostname;
const hostname = new URLParse(activeUrl).hostname;
const permittedAcc = getPermittedAccountsByHostname(
permissionsControllerState,
hostname,
);
return permittedAcc;
}, isEqual);

const favicon = useFavicon(activeUrl.current);
const favicon = useFavicon(activeUrl);
const { trackEvent, isEnabled, getMetaMetricsId, createEventBuilder } =
useMetrics();
/**
Expand All @@ -177,25 +178,31 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
/**
* Checks if a given url or the current url is the homepage
*/
const isHomepage = useCallback((checkUrl = null) => {
const currentPage = checkUrl || activeUrl.current;
const prefixedUrl = prefixUrlWithProtocol(currentPage);
const { host: currentHost } = getUrlObj(prefixedUrl);
return (
currentHost === HOMEPAGE_HOST || currentHost === OLD_HOMEPAGE_URL_HOST
);
}, []);
const isHomepage = useCallback(
(checkUrl = null) => {
const currentPage = checkUrl || activeUrl;
const prefixedUrl = prefixUrlWithProtocol(currentPage);
const { host: currentHost } = getUrlObj(prefixedUrl);
return (
currentHost === HOMEPAGE_HOST || currentHost === OLD_HOMEPAGE_URL_HOST
);
},
[activeUrl],
);

const notifyAllConnections = useCallback((payload) => {
const fullHostname = new URLParse(activeUrl.current).hostname;
const notifyAllConnections = useCallback(
(payload) => {
const fullHostname = new URLParse(activeUrl).hostname;

// TODO:permissions move permissioning logic elsewhere
backgroundBridges.current.forEach((bridge) => {
if (bridge.hostname === fullHostname) {
bridge.sendNotification(payload);
}
});
}, []);
// TODO:permissions move permissioning logic elsewhere
backgroundBridges.current.forEach((bridge) => {
if (bridge.hostname === fullHostname) {
bridge.sendNotification(payload);
}
});
},
[activeUrl],
);

/**
* Dismiss the text selection on the current website
Expand Down Expand Up @@ -246,6 +253,8 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
toggleOptionsIfNeeded();
const { current } = webviewRef;
current && current.goBack();

// we need to update the activeUrl state to the previous url
}, [backEnabled, toggleOptionsIfNeeded]);

/**
Expand Down Expand Up @@ -480,8 +489,8 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
const { current } = webviewRef;

current && current.reload();
triggerDappViewedEvent(activeUrl.current);
}, []);
triggerDappViewedEvent(activeUrl);
}, [activeUrl]);

/**
* Handle when the drawer (app menu) is opened
Expand Down Expand Up @@ -592,7 +601,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
title: string;
icon: ImageSourcePropType;
}) => {
activeUrl.current = siteInfo.url;
setActiveUrl(siteInfo.url);
title.current = siteInfo.title;
if (siteInfo.icon) icon.current = siteInfo.icon;
};
Expand Down Expand Up @@ -712,7 +721,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
webStates.current[url] = { ...webStates.current[url], ended: true };
const { started, ended } = webStates.current[url];
const incomingOrigin = new URLParse(url).origin;
const activeOrigin = new URLParse(activeUrl.current).origin;
const activeOrigin = new URLParse(activeUrl).origin;
if ((started && ended) || incomingOrigin === activeOrigin) {
delete webStates.current[url];
// Update navigation bar address with title of loaded url.
Expand Down Expand Up @@ -760,10 +769,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
}
} catch (e: unknown) {
const onMessageError = e as Error;
Logger.error(
onMessageError,
`Browser::onMessage on ${activeUrl.current}`,
);
Logger.error(onMessageError, `Browser::onMessage on ${activeUrl}`);
}
};

Expand All @@ -772,7 +778,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
*/
const goToHomepage = async () => {
toggleOptionsIfNeeded();
if (activeUrl.current === HOMEPAGE_URL) return reload();
if (activeUrl === HOMEPAGE_URL) return reload();
await go(HOMEPAGE_URL);
trackEvent(createEventBuilder(MetaMetricsEvents.DAPP_HOME).build());
};
Expand Down Expand Up @@ -801,7 +807,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
getProviderState,
navigation,
// Website info
url: activeUrl,
url: { current: activeUrl },
title,
icon,
// Bookmarks
Expand Down Expand Up @@ -849,7 +855,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {

// Cancel loading the page if we detect its a phishing page
if (!isAllowedOrigin(urlOrigin)) {
handleNotAllowedUrl(activeUrl.current); // should this be activeUrl.current instead of url?
handleNotAllowedUrl(activeUrl); // should this be activeUrl instead of url?
Copy link
Contributor

@joaoloureirop joaoloureirop Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

good question. At this point, activeUrl value should be the same as urlOrigin value.

It would make more sense to use urlOrigin

return false;
}

Expand Down Expand Up @@ -996,9 +1002,9 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
);

const checkTabPermissions = useCallback(() => {
if (!activeUrl.current) return;
if (!activeUrl) return;

const hostname = new URLParse(activeUrl.current).hostname;
const hostname = new URLParse(activeUrl).hostname;
const permissionsControllerState =
Engine.context.PermissionController.state;
const permittedAccounts = getPermittedAccountsByHostname(
Expand Down Expand Up @@ -1042,7 +1048,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
Logger.error(checkTabPermissionsError, 'Error in checkTabPermissions');
}
}
}, [props.chainId, navigation]);
}, [props.chainId, navigation, activeUrl]);

useEffect(() => {
if (
Expand Down Expand Up @@ -1080,7 +1086,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {

const onCancel = () => {
// Reset the url bar to the current url
urlBarRef.current?.setNativeProps({ text: activeUrl.current });
urlBarRef.current?.setNativeProps({ text: activeUrl });
};

const onFocusUrlBar = () => {
Expand Down Expand Up @@ -1115,7 +1121,7 @@ export const BrowserTab: React.FC<BrowserTabProps> = (props) => {
onBlur={onBlurUrlBar}
onChangeText={onChangeUrlBar}
connectedAccounts={permittedAccountsList}
activeUrlRef={activeUrl}
activeUrl={activeUrl}
/>
<View style={styles.wrapper}>
{renderProgressBar()}
Expand Down
16 changes: 9 additions & 7 deletions app/components/Views/BrowserTab/components/Options/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ interface OptionsProps {
toggleOptions: () => void;
onNewTabPress: () => void;
toggleOptionsIfNeeded: () => void;
activeUrl: MutableRefObject<string>;
activeUrl: string;
isHomepage: () => boolean;
getMaskedUrl: (urlToMask: string, sessionENSNames: SessionENSNames) => string;
onSubmitEditing: (url: string) => void;
Expand Down Expand Up @@ -83,9 +83,9 @@ const Options = ({
*/
const openInBrowser = () => {
toggleOptionsIfNeeded();
Linking.openURL(activeUrl.current).catch((openInBrowserError) =>
Linking.openURL(activeUrl).catch((openInBrowserError) =>
Logger.log(
`Error while trying to open external link: ${activeUrl.current}`,
`Error while trying to open external link: ${activeUrl}`,
openInBrowserError,
),
);
Expand All @@ -98,7 +98,7 @@ const Options = ({
*/
const goToFavorites = async () => {
toggleOptionsIfNeeded();
if (activeUrl.current === OLD_HOMEPAGE_URL_HOST) return reload();
if (activeUrl === OLD_HOMEPAGE_URL_HOST) return reload();
await onSubmitEditing(OLD_HOMEPAGE_URL_HOST);
trackEvent(
createEventBuilder(MetaMetricsEvents.DAPP_GO_TO_FAVORITES).build(),
Expand Down Expand Up @@ -127,7 +127,7 @@ const Options = ({
screen: 'AddBookmark',
params: {
title: title.current || '',
url: getMaskedUrl(activeUrl.current, sessionENSNames),
url: getMaskedUrl(activeUrl, sessionENSNames),
onAddBookmark: async ({
name,
url: urlToAdd,
Expand Down Expand Up @@ -197,7 +197,9 @@ const Options = ({
};

const isBookmark = () => {
const maskedUrl = getMaskedUrl(activeUrl.current, sessionENSNames);
const maskedUrl = getMaskedUrl(activeUrl, sessionENSNames);
console.log('ENTER maskedUrl', maskedUrl);

return bookmarks.some(
({ url: bookmark }: { url: string }) => bookmark === maskedUrl,
);
Expand All @@ -218,7 +220,7 @@ const Options = ({
const share = () => {
toggleOptionsIfNeeded();
Share.open({
url: activeUrl.current,
url: activeUrl,
}).catch((err) => {
Logger.log('Error while trying to share address', err);
});
Expand Down
6 changes: 3 additions & 3 deletions app/components/Views/BrowserTab/components/PhishingModal/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ interface PhishingModalProps {
go: (url: string) => void;
urlBarRef: React.MutableRefObject<TextInput | null>;
addToWhitelist: (hostname: string) => void;
activeUrl: React.MutableRefObject<string>;
activeUrl: string;
blockListType: React.MutableRefObject<string>;
onSubmitEditing: (url: string) => void;
}
Expand Down Expand Up @@ -59,7 +59,7 @@ const PhishingModal = ({
addToWhitelist(urlOrigin);
setShowPhishingModal(false);

blockedUrl !== activeUrl.current &&
blockedUrl !== activeUrl &&
setTimeout(() => {
onSubmitEditing(blockedUrl);
setBlockedUrl(undefined);
Expand Down Expand Up @@ -88,7 +88,7 @@ const PhishingModal = ({
* Go back from phishing website alert
*/
const goBackToSafety = () => {
urlBarRef.current?.setNativeProps({ text: activeUrl.current });
urlBarRef.current?.setNativeProps({ text: activeUrl });

setTimeout(() => {
setShowPhishingModal(false);
Expand Down
Loading