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.

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

Convert Connected Sites page to modal #8254

Merged
merged 2 commits into from
Mar 30, 2020

Conversation

whymarrh
Copy link
Contributor

@whymarrh whymarrh commented Mar 30, 2020

#8242 and #8253 was the preparation work.

This PR migrates the existing Connected Sites page to a modal, serving as the base for the new modal designs (in a forthcoming PR). This leaves the routing intact, i.e. the #connected-sites route still exists & works, but moves the route into the Home component so that the home screen is the backdrop.

Screenshots:

This commit updates the existing _Connected Sites_ section to a modal using
the `Popover` component. This will serve as a base for the new modal design.
@@ -163,6 +163,7 @@ describe('MetaMask', function () {
await driver.switchToWindow(extension)

await driver.clickElement(By.xpath(`//button[contains(text(), 'Disconnect All')]`))
await driver.clickElement(By.css('.popover-bg'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The confirmation modal is the old-style modal and is thus behind the popover. This is a workaround pending the forthcoming redesign.

@whymarrh whymarrh force-pushed the connected-sites-modal branch from 61d0127 to 256ad92 Compare March 30, 2020 23:04
@@ -163,6 +163,7 @@ describe('MetaMask', function () {
await driver.switchToWindow(extension)

await driver.clickElement(By.xpath(`//button[contains(text(), 'Disconnect All')]`))
await driver.clickElement(By.css('.popover-header__close'))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The confirmation modal is the old-style modal and is thus behind the popover. This is a workaround pending the forthcoming redesign.

Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@whymarrh whymarrh merged commit d82f06c into MetaMask:develop Mar 30, 2020
@whymarrh whymarrh deleted the connected-sites-modal branch March 30, 2020 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants