<p><b>@ManfredKarrer</b> requested changes on this pull request.</p>

<p>NACK<br>
I agree that it is good to add a cancel button here but it must be done differently.</p>
<p>The WalletPasswordWindow is used in several places and the close button must not trigger a shitdonw in the other use cases.<br>
The hideCloseButton call can be removed in MainViewModel at line294.<br>
To add the shut down functionality need to be added there as well so it does not affect the other use cases.</p>
<p><code>bisqSetup.setRequestWalletPasswordHandler(aesKeyHandler -> walletPasswordWindow .onAesKey(aesKeyHandler::accept) .onClose(()-> BisqApp.getShutDownHandler().run()) .show());</code></p>
<p>Should do it.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/bisq-network/bisq/pull/1765#pullrequestreview-162676088">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AkpZtqolQXQlpfNIvNLmHDTVUMtdQRPZks5ui-SAgaJpZM4XNzzI">mute the thread</a>.<img src="https://github.com/notifications/beacon/AkpZtn8dAaUdZzO6STxTR9Xwcb1vJrujks5ui-SAgaJpZM4XNzzI.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/bisq-network/bisq","title":"bisq-network/bisq","subtitle":"GitHub repository","main_image_url":"https://assets-cdn.github.com/images/email/message_cards/header.png","avatar_image_url":"https://assets-cdn.github.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/bisq-network/bisq"}},"updates":{"snippets":[{"icon":"PERSON","message":"@ManfredKarrer requested changes on #1765"}],"action":{"name":"View Pull Request","url":"https://github.com/bisq-network/bisq/pull/1765#pullrequestreview-162676088"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/1765#pullrequestreview-162676088",
"url": "https://github.com/bisq-network/bisq/pull/1765#pullrequestreview-162676088",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
},
{
"@type": "MessageCard",
"@context": "http://schema.org/extensions",
"hideOriginalBody": "false",
"originator": "AF6C5A86-E920-430C-9C59-A73278B5EFEB",
"title": "@ManfredKarrer requested changes on 1765",
"sections": [
{
"text": "NACK\r\nI agree that it is good to add a cancel button here but it must be done differently.\r\n\r\nThe WalletPasswordWindow is used in several places and the close button must not trigger a shitdonw in the other use cases. \r\nThe hideCloseButton call can be removed in MainViewModel at line294.\r\nTo add the shut down functionality need to be added there as well so it does not affect the other use cases.\r\n\r\n`  bisqSetup.setRequestWalletPasswordHandler(aesKeyHandler -\u003e walletPasswordWindow\r\n                .onAesKey(aesKeyHandler::accept)\r\n                .onClose(()-\u003e BisqApp.getShutDownHandler().run())\r\n                .show());`\r\n\r\nShould do it.",
"activityTitle": "**Manfred Karrer**",
"activityImage": "https://assets-cdn.github.com/images/email/message_cards/avatar.png",
"activitySubtitle": "@ManfredKarrer",
"facts": [

]
}
],
"potentialAction": [
{
"targets": [
{
"os": "default",
"uri": "https://github.com/bisq-network/bisq/pull/1765#pullrequestreview-162676088"
}
],
"@type": "OpenUri",
"name": "View on GitHub"
},
{
"name": "Unsubscribe",
"@type": "HttpPOST",
"target": "https://api.github.com",
"body": "{\n\"commandName\": \"MuteNotification\",\n\"threadId\": 389496008\n}"
}
],
"themeColor": "26292E"
}
]</script>