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

Jump to bottom

Add remove queued channel button #48

Merged
merged 2 commits into from
Nov 7, 2022
Merged

Add remove queued channel button #48

merged 2 commits into from
Nov 7, 2022

Conversation

0xBEEFCAF3
Copy link
Contributor

No description provided.

@0xBEEFCAF3
Copy link
Contributor Author

Follow up PR will include a get recommended channels and reset all queued channels button

DanGould
DanGould reviewed Nov 4, 2022
View reviewed changes
static/index.html Show resolved Hide resolved
static/index.html Outdated Show resolved Hide resolved
static/index.html Outdated
Comment on lines 89 to 90
if (table.children.length <= 2) {
return
}
Copy link
Contributor

Choose a reason for hiding this comment

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

disable the button on this check too?

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 think the UX is a bit off if the button becomes disabled after a click. Ideally we want to have it disabled on init render. I think we can address this in a later PR.

static/index.html Outdated Show resolved Hide resolved
static/index.html Outdated Show resolved Hide resolved
@0xBEEFCAF3 0xBEEFCAF3 force-pushed the remove-button branch 2 times, most recently from 92037ed to 8520171 Compare November 4, 2022 16:59
@0xBEEFCAF3 0xBEEFCAF3 requested a review from DanGould November 5, 2022 20:40
@0xBEEFCAF3
Copy link
Contributor Author

@DanGould Can you take another pass at this PR. Would like to get it merged soon

@DanGould
Copy link
Contributor

DanGould commented Nov 6, 2022

I made the js stateless & fixed button content

@DanGould
Copy link
Contributor

DanGould commented Nov 6, 2022

The test was looping infinitely on Attempting to contact btcrpc: JSON-RPC error: transport error: Couldn't connect to host: Connection refused (os error 111)

that's a timeout issue afaik, not an issue with this pr

@DanGould
Copy link
Contributor

DanGould commented Nov 6, 2022 •
edited
Loading

@0xBEEFCAF3 do we have your approval to merge?

@0xBEEFCAF3
Copy link
Contributor Author

@0xBEEFCAF3 do we have your approval to merge?

I don't :(

@DanGould
Copy link
Contributor

DanGould commented Nov 7, 2022

you don't approve of the changes???

@0xBEEFCAF3
Copy link
Contributor Author

0xBEEFCAF3 commented Nov 7, 2022 •
edited
Loading

I thought you asked if I can merge. Feel free to merge when you think it's ready.

@DanGould DanGould merged commit 99d6111 into payjoin:master Nov 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanGould DanGould DanGould approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@0xBEEFCAF3 @DanGould