Skip to content

Opening Network Forms in full screen from the popup view #15442

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

Merged
merged 1 commit into from
Aug 3, 2022
+2 −4
Merged

Opening Network Forms in full screen from the popup view #15442

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 2 additions & 4 deletions ui/pages/settings/networks-tab/networks-list-item/networks-list-item.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,14 @@ import React, { useEffect, useRef } from 'react';
import PropTypes from 'prop-types';
import classnames from 'classnames';
import { useDispatch, useSelector } from 'react-redux';
import { useHistory } from 'react-router-dom';
import { useI18nContext } from '../../../../hooks/useI18nContext';
import {
NETWORK_TYPE_RPC,
CHAIN_ID_TO_NETWORK_IMAGE_URL_MAP,
} from '../../../../../shared/constants/network';
import LockIcon from '../../../../components/ui/lock-icon';
import IconCheck from '../../../../components/ui/icon/icon-check';
import { NETWORKS_FORM_ROUTE } from '../../../../helpers/constants/routes';
import { NETWORKS_ROUTE } from '../../../../helpers/constants/routes';
import { setSelectedSettingsRpcUrl } from '../../../../store/actions';
import { getEnvironmentType } from '../../../../../app/scripts/lib/util';
import { ENVIRONMENT_TYPE_FULLSCREEN } from '../../../../../shared/constants/app';
Expand All @@ -28,7 +27,6 @@ const NetworksListItem = ({
setSearchedNetworks,
}) => {
const t = useI18nContext();
const history = useHistory();
const dispatch = useDispatch();
const environmentType = getEnvironmentType();
const isFullScreen = environmentType === ENVIRONMENT_TYPE_FULLSCREEN;
Expand Down Expand Up @@ -68,7 +66,7 @@ const NetworksListItem = ({
setSearchedNetworks([]);
dispatch(setSelectedSettingsRpcUrl(rpcUrl));
if (!isFullScreen) {
history.push(NETWORKS_FORM_ROUTE);
global.platform.openExtensionInBrowser(NETWORKS_ROUTE);
Copy link
Contributor

@danjm danjm Aug 3, 2022

Choose a reason for hiding this comment

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

Why NETWORKS_ROUTE instead of NETWORKS_FORM_ROUTE?

Copy link
Contributor

@digiwand digiwand Aug 3, 2022

Choose a reason for hiding this comment

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

const NETWORKS_ROUTE = '/settings/networks';
const NETWORKS_FORM_ROUTE = '/settings/networks/form';

Went down a bit of a rabbit hole here. As the issue shows, the route, settings/networks/form, resolves to the generic settings page. From my investigation, it appears the path is not resolving to the networks tab properly and from there, the shouldRenderNetworkForm value when the route is NETWORKS_FORM_ROUTE is not being set.

hardcoding the route to resolve to the networks tab and setting shouldRenderNetworkForm to true shows this popup page:
Screenshot 2022-08-03 at 8 53 17 PM

I think that applying the current changes now would make NETWORKS_FORM_ROUTE obsolete which might not be what we want.

p.s. i deleted the last copy of this comment and reposted this comment with the proper reference to see if I can remove the reference to the incorrect issue ticket

Copy link
Contributor

@digiwand digiwand Aug 4, 2022

Choose a reason for hiding this comment

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

p.s.p.s. the reference to this PR from the unrelated issue was not removed when I deleted the comment I mentioned above

}
}}
>
Expand Down