<p></p>
<p><b>@ripcurlx</b> requested changes on this pull request.</p>
<p>NACK - Please see my comments.</p>
<p>Additionally I recognized following issues:</p>
<p><a target="_blank" rel="noopener noreferrer" href="https://user-images.githubusercontent.com/170962/122949761-3562b980-d37c-11eb-8aa5-ccca95f69a7b.png"><img src="https://user-images.githubusercontent.com/170962/122949761-3562b980-d37c-11eb-8aa5-ccca95f69a7b.png" alt="Bildschirmfoto 2021-06-22 um 17 02 13" style="max-width:100%;"></a></p>
<ul>
<li>Based on the designs by <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/users/pedromvpg/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/pedromvpg">@pedromvpg</a> the title should be bold.</li>
<li>Right now it is not possible to click on the learn more link as it is hidden as soon as you try to hover over the tooltip.</li>
<li>It would be great to add the suggestion by <a class="user-mention" data-hovercard-type="user" data-hovercard-url="/users/pedromvpg/hovercard" data-octo-click="hovercard-link-click" data-octo-dimensions="link_type:self" href="https://github.com/pedromvpg">@pedromvpg</a> <code>Also the user should be told how many days are left after the first signing.</code> as well.</li>
</ul>
<p><a target="_blank" rel="noopener noreferrer" href="https://user-images.githubusercontent.com/170962/122950914-2597a500-d37d-11eb-9241-1c1a625e2e5d.png"><img src="https://user-images.githubusercontent.com/170962/122950914-2597a500-d37d-11eb-9241-1c1a625e2e5d.png" alt="Bildschirmfoto 2021-06-22 um 17 12 55" style="max-width:100%;"></a></p>
<p>It would be good to use the same tooltip in the accounts section as well.</p>
<p>Besides that everything is looking promising <g-emoji class="g-emoji" alias="smile" fallback-src="https://github.githubassets.com/images/icons/emoji/unicode/1f604.png">😄</g-emoji></p><hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/5569#discussion_r656299772">desktop/src/main/java/bisq/desktop/components/AccountStatusTooltipLabel.java</a>:</p>
<pre style='color:#555'>> + public AccountStatusTooltipLabel(OfferBookListItem.WitnessAgeData witnessAgeData,
+ CoinFormatter formatter) {
+ super(witnessAgeData.getDisplayString());
+ this.witnessAgeData = witnessAgeData;
+ this.textIcon = FormBuilder.getIcon(witnessAgeData.getIcon());
+ this.popupTitle = witnessAgeData.isLimitLifted()
+ ? Res.get("offerbook.timeSinceSigning.tooltip.accountLimitLifted")
+ : Res.get("offerbook.timeSinceSigning.tooltip.accountLimit", formatter.formatCoinWithCode(OfferRestrictions.TOLERATED_SMALL_TRADE_AMOUNT));
+
+ positionAndActivateIcon();
+ }
+
+ private void positionAndActivateIcon() {
+ textIcon.setOpacity(0.4);
+ textIcon.getStyleClass().add("tooltip-icon");
+ textIcon.setOnMouseEntered(e -> popoverWrapper.showPopOver(() -> createPopOver()));
</pre>
⬇️ Suggested change
<pre style="color: #555">- textIcon.setOnMouseEntered(e -> popoverWrapper.showPopOver(() -> createPopOver()));
+ textIcon.setOnMouseEntered(e -> popoverWrapper.showPopOver(this::createPopOver));
</pre>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/5569#discussion_r656300154">desktop/src/main/java/bisq/desktop/components/AccountStatusTooltipLabel.java</a>:</p>
<pre style='color:#555'>> + learnMoreLink.setOnAction((e) -> {
+ GUIUtil.openWebPage("https://bisq.wiki/Account_limits");
+ });
</pre>
⬇️ Suggested change
<pre style="color: #555">- learnMoreLink.setOnAction((e) -> {
- GUIUtil.openWebPage("https://bisq.wiki/Account_limits");
- });
+ learnMoreLink.setOnAction((e) -> GUIUtil.openWebPage("https://bisq.wiki/Account_limits"));
</pre>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/5569#discussion_r656310825">core/src/main/resources/i18n/displayStrings.properties</a>:</p>
<pre style='color:#555'>> @@ -354,6 +354,14 @@ offerbook.timeSinceSigning.info.signer=signed by peer and can sign peer accounts
offerbook.timeSinceSigning.info.banned=account was banned
offerbook.timeSinceSigning.daysSinceSigning={0} days
offerbook.timeSinceSigning.daysSinceSigning.long={0} since signing
+offerbook.timeSinceSigning.tooltip.accountLimit=Account limit: {0}
+offerbook.timeSinceSigning.tooltip.accountLimitLifted=Account limit lifted
+offerbook.timeSinceSigning.tooltip.info.unsigned=This account hasn't been signed yet
+offerbook.timeSinceSigning.tooltip.info.signed=This account has been signed
+offerbook.timeSinceSigning.tooltip.info.signedAndLifted=This account has been signed and can sign peer accounts
+offerbook.timeSinceSigning.tooltip.checkmark.buyBtc=but BTC from a signed account
</pre>
⬇️ Suggested change
<pre style="color: #555">-offerbook.timeSinceSigning.tooltip.checkmark.buyBtc=but BTC from a signed account
+offerbook.timeSinceSigning.tooltip.checkmark.buyBtc=buy BTC from a signed account
</pre>
<hr>
<p>In <a href="https://github.com/bisq-network/bisq/pull/5569#discussion_r656314005">desktop/src/main/java/bisq/desktop/bisq.css</a>:</p>
<pre style='color:#555'>> @@ -31,6 +31,10 @@
-fx-text-fill: -bs-rd-error-red;
}
+.text-gray-ddd {
</pre>
<p>I think it is better not to include specific color codes in the css class name as it might be different colors in different themes.</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/5569#pullrequestreview-689615469">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AJFFTNVBYIYNT32EJPI6P4DTUCSJTANCNFSM46RFC7EQ">unsubscribe</a>.<img src="https://github.com/notifications/beacon/AJFFTNSOGHIAJLU5NPS6ESTTUCSJTA5CNFSM46RFC7E2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOFENLE3I.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/bisq-network/bisq/pull/5569#pullrequestreview-689615469",
"url": "https://github.com/bisq-network/bisq/pull/5569#pullrequestreview-689615469",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>