Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

fix(ui): close channel create modal after channel opening is finished #2923

Merged
merged 3 commits into from
Oct 7, 2019

Conversation

korhaliv
Copy link
Member

@korhaliv korhaliv commented Oct 3, 2019

Description:

Close Create channel modal after channel opening is finished (either successful or not). This functionality was broken some time during Modal Stack refactoring. Also remove redundant openChannel.data handler which was causing comlink error.

    grpc.services.Lightning.once(	
      'openChannel.data',	
      proxyValue(data => dispatch(pushchannelupdated(data)))	
    )

Is redundant because openChannel call resolves with exactly same data as per

this.emit('openChannel.data', response)

Fix #2920

How Has This Been Tested?

Manually

Types of changes:

Bugfix

Checklist:

  • My code follows the code style of this project.
  • I have reviewed and updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes where needed.
  • All new and existing tests passed.
  • My commits have been squashed into a concise set of changes.

@korhaliv korhaliv added the type: bug 🐛 Something isn't working label Oct 3, 2019
@korhaliv korhaliv added this to the v0.6.0-beta milestone Oct 3, 2019
@korhaliv korhaliv requested a review from mrfelton October 3, 2019 13:06
@korhaliv korhaliv self-assigned this Oct 3, 2019
@coveralls
Copy link

coveralls commented Oct 3, 2019

Coverage Status

Coverage increased (+0.008%) to 23.016% when pulling 1984974 on korhaliv:fix/channel-open into a40c13d on LN-Zap:next.

Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

As per slack discussion, lets remove the unused events and do the same for closeChannel

@korhaliv
Copy link
Member Author

korhaliv commented Oct 7, 2019

@mrfelton updated

@korhaliv korhaliv requested a review from mrfelton October 7, 2019 13:28
Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Minor thing here - can you ensure that each fix commit has a scope please for the changelogs. Thanks! missing from 1st commit in this PR.

@korhaliv korhaliv requested a review from mrfelton October 7, 2019 16:49
@korhaliv
Copy link
Member Author

korhaliv commented Oct 7, 2019

@mrfelton updated

Copy link
Member

@mrfelton mrfelton left a comment

Choose a reason for hiding this comment

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

Tested ACK 1984974

@mrfelton mrfelton merged commit 9993e38 into LN-Zap:next Oct 7, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
type: bug 🐛 Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants