Skip to content
This repository has been archived by the owner. It is now read-only.

Hides lion badge when count is 0 #7877

Merged
merged 1 commit into from Mar 25, 2017
Merged

Conversation

@NejcZdovc
Copy link
Member

NejcZdovc commented Mar 24, 2017

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Resolves #7873

Auditors

@bsclifton @luixxiul

Test Plan

  • go to brave.com
  • badge should be hidden
Resolves #7873

Auditors: @bsclifton @luixxiul

Test Plan:
- go to brave.com
- badge should be hidden
@NejcZdovc NejcZdovc added this to the 0.14.0 milestone Mar 24, 2017
@NejcZdovc NejcZdovc self-assigned this Mar 24, 2017
@NejcZdovc NejcZdovc requested review from luixxiul, bsclifton and srirambv Mar 24, 2017
@jonathansampson
Copy link
Collaborator

jonathansampson commented Mar 25, 2017

LGTM

Copy link
Member

bsclifton left a comment

Feedback left- maybe when looking at the fingerprinting, we can do the tests 😄

@@ -886,7 +886,7 @@ class Main extends ImmutableComponent {
const trackers = frames.getIn(['trackingProtection', 'blocked'])
const blocked = (ads ? ads.size : 0) + (trackers ? trackers.size : 0)

return (blocked > 99) ? '99+' : blocked
return (blocked.size === 0) ? false : ((blocked > 99) ? '99+' : blocked)

This comment has been minimized.

Copy link
@bsclifton

bsclifton Mar 25, 2017

Member

The change looks great. I think it could be better though if you pulled the logic out to a method and then had a unit test for it, using enzyme 😄

@bsclifton bsclifton merged commit 577f9b6 into brave:master Mar 25, 2017
0 of 2 checks passed
0 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
continuous-integration/appveyor/pr AppVeyor build failed
Details
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.