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

Feature/fullscreen btn #1580

Open
wants to merge 13 commits into
base: release/5.25.0
Choose a base branch
from
Open

Conversation

piyalbasu
Copy link
Contributor

@piyalbasu piyalbasu commented Oct 24, 2024

This PR adds a button to open up the extension in the user's browser. Visually, most of the views look okay in fullscreen mode, but there are some instances where a user might run into issues with stale data. For example, if a user opens Freighter in a new tab, and then leaves and submits some other tx's in a different tab, when they come back to the Freighter tab, their balances could be out of date. In a follow up PR, I'm going to explore solutions for solving this (maybe polling? maybe throwing up a warning saying "please refresh the app, you've been idle for too long")

We've always counted on user session's being pretty short since the app closes when a user clicks out of the extension. This may not be the case with this feature

@piyalbasu
Copy link
Contributor Author

Screenshot 2024-10-24 at 12 56 09 PM Screenshot 2024-10-24 at 12 56 18 PM

This PR incorporates #1558 and #1548

await page.getByText("Manage Assets").click({ force: true });
await page.getByPlaceholder("Enter password").fill(PASSWORD);
await page.getByText("Log In").click({ force: true });
await expect(page.getByText("Your assets")).toBeVisible();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes a bug with snapshot: https://github.com/stellar/freighter/pull/1580/files#diff-93b044b86208b123f27589535905ad725da70f6b467ee0559c988a9f866e71e1

It was mistakenly snapshotting the password confirm screen instead of the manage assets screen because it wasn't waiting for the view to load

.AccountOptionsDropdown {
&__modal {
background: var(--sds-clr-gray-01);
border-radius: #{pxToRem(6px)};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm trying to make a conscious effort to start using this pxToRem helper more. It will probably make our css a little more foolproof if we let the helper do the math for us

@aristidesstaffieri
Copy link
Contributor

This PR adds a button to open up the extension in the user's browser. Visually, most of the views look okay in fullscreen mode, but there are some instances where a user might run into issues with stale data. For example, if a user opens Freighter in a new tab, and then leaves and submits some other tx's in a different tab, when they come back to the Freighter tab, their balances could be out of date. In a follow up PR, I'm going to explore solutions for solving this (maybe polling? maybe throwing up a warning saying "please refresh the app, you've been idle for too long")

We've always counted on user session's being pretty short since the app closes when a user clicks out of the extension. This may not be the case with this feature

Yeah I'm not sure if users expect the UI to be a live feed or not. I think this problem exists even without full screen mode though because you can open your Freighter extension and then through some other means change the state of that account and the extension will also not poll or do anything to refresh balance without a user navigation.

I guess maybe though we expect tabs could hang around longer than an open extension popup.

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