You are viewing a single comment's thread from:

RE: ULOG #20: Adding Announcement Banner + Extra Monetization

in #ulog5 years ago (edited)

Thank you for your contribution. The post is of high quality as usual, with details about the changes, main code changes and lesson learned. Announcement Banner is actually a nice little feature to have in any application. Also whenever we do add some feature, it's always better to find a way to do some code refactoring.

What will happen if the displayBanner2 or displayBanner1 is false, even though it is false you showing the alert or am I missing something. https://github.com/surpassinggoogle/UlogsV2/pull/229/files#diff-105a0ee4a79800ce929a0e78481a9286R252


If you would like further explanation of the given score, please ask.


Your contribution has been evaluated according to Utopian policies and guidelines, as well as a predefined set of questions pertaining to the category.

To view those questions and the relevant answers related to your post, click here.


Need help? Chat with us on Discord.

[utopian-moderator]

Sort:  

Thank you for the feedback and code review @codingdefined.

What will happen if the displayBanner2 or displayBanner1 is false, even though it is false you showing the alert or am I missing something.

You're right, I missed that as well. There were multiple versions of this implementation and the last version I committed was not that polished.

What I had in mind was that if any of the banner flags were set, the banner should be displayed. But if only one was set, then a one liner banner should be displayed. And if both flags are false, then hide the banner. I'm working on this now.

The benefit of peer reviews.

That's great to know, Cheers !!!

Thank you for your review, @codingdefined! Keep up the good work!

Coin Marketplace

STEEM 0.27
TRX 0.12
JST 0.031
BTC 57654.82
ETH 2888.30
USDT 1.00
SBD 3.60