Skip to content

Add Loop History page #39

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

Merged
merged 5 commits into from
May 19, 2020
Merged

Add Loop History page #39

merged 5 commits into from
May 19, 2020

Conversation

jamaljsr
Copy link
Member

Closes #25

Note: This branch is built on top of PR #38, so only the last 4 commits should be reviewed here.

This PR adds the Loop History page to display a table of all the swaps executed on the node. There are two ways to get to the History page: from the left navbar, and from the clock icon at the top of the Loop page

Steps to test:

  1. Submit a few swaps, leaving one or more pending
  2. Click on the History link in the nav bar
  3. Confirm that you can see a list of the swaps you submitted
  4. Verify that the information in the columns looks correct
  5. Click on the left arrow on the top of the page to go back to the Loop page
  6. Click on the clock icon on the top right of the page to go back to the History page

@jamaljsr jamaljsr requested a review from guggero May 16, 2020 16:32
Copy link
Member

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Very nice, LGTM 🎉
Tested it locally with the GrUB and it's really cool to see the status updating automatically.
What still gets me is the "slow" swap that happens by default. But I'm sure the option to create a fast swap from the UI is coming soon?

<Column>{swap.typeName}</Column>
<Column>{swap.amount.toLocaleString()}</Column>
<Column right>
{swap.createdOn.toLocaleDateString()} {swap.createdOn.toLocaleTimeString()}
Copy link
Member

Choose a reason for hiding this comment

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

Not blocking at all but would be nice if we showed something like <1 minute ago here and only show the full time stamp on mouse over as a title="" tooltip. Would also save some space.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I thought about doing this as well, but I wasn't sure what our users would prefer. In this tabular format, I personally rather see exact timestamps than the vague "X time ago" format, but agree the latter is more user friendly. I am fine with changing it based on what others on the team think. I'll bring it up in the next product meeting.

Regarding the "slow" swap, this is happening because you are packaging a production build of the app into the binary. In loop.ts, we only use the "fast" swap in the dev environment.

We should think about how we can run the app+grub in development mode, because it will also be a challenge to work on the app if I have to "make build" after every line change to see it reflected in the browser.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I guess it's mostly personal preference. I'm happy to leave it as is if that's the preferred way.

Ah, I see. Maybe we should make those settings dependent on the chain rather than the nodejs build env? Do an initial GetInfo call and store the chain in the store? Then always use fast for regtest. If that's not too tricky to do or requires a big refactor of how the store works.

You can still use the grub only as a backend/proxy. It should work if you start the UI separately and just point it to 8443.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great idea about using the chain to decide how fast to go. I made a note of it and will make the change.

Copy link
Member Author

Choose a reason for hiding this comment

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

FYI: I cannot run the UI separately right now. Im getting a CORS error. I do see CORS should be disabled on the proxy server in buildGrpcProxyServer so I'm not sure why the browser is throwing an error. It's not a blocker right now, just wanted to mention it.

Access to fetch at 'https://localhost:8443/lnrpc.Lightning/ListChannels' from origin 'http://localhost:3000' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: No 'Access-Control-Allow-Origin' header is present on the requested resource. If an opaque response serves your needs, set the request's mode to 'no-cors' to fetch the resource with CORS disabled.

VM5907:1 POST https://localhost:8443/lnrpc.Lightning/ListChannels net::ERR_FAILED

Copy link
Member

Choose a reason for hiding this comment

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

Okay, thanks for the information. I still need to cleanup the grub and test some of the features.

@jamaljsr jamaljsr force-pushed the feat/loop-history branch from 5268036 to 72f343c Compare May 19, 2020 15:05
@jamaljsr jamaljsr merged commit 7dad4b7 into master May 19, 2020
@jamaljsr jamaljsr deleted the feat/loop-history branch May 19, 2020 15:13
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.

Create Loop History page
2 participants