Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Resolves missing browserAction icon #10197

Merged
merged 1 commit into from
Jul 30, 2017
Merged

Resolves missing browserAction icon #10197

merged 1 commit into from
Jul 30, 2017

Conversation

jonathansampson
Copy link
Collaborator

Submitter Checklist:

  • 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).
  • Tagged reviewers and labelled the pull request as needed.

Fixes #9437

Test Plan:

  1. Enable bitwarden
  2. Navigate to google.com

Ensure that bitwarden browserAction icon is visible.

Reviewer Checklist:

Tests

  • Adequate test coverage exists to prevent regressions
  • Tests should be independent and work correctly when run individually or as a suite ref
  • New files have MPL2 license header

@jonathansampson jonathansampson added this to the 0.18.x Hotfix milestone Jul 29, 2017
Copy link
Contributor

@NejcZdovc NejcZdovc left a comment

Choose a reason for hiding this comment

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

please see my comment here #9437 (comment). I think it would be better to override browserActions with new tabAction data then other way around. This way if browserActions and tabAction have the same value we would use tabAction's value.

@codecov-io
Copy link

codecov-io commented Jul 29, 2017

Codecov Report

Merging #10197 into master will increase coverage by <.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master   #10197      +/-   ##
==========================================
+ Coverage   52.78%   52.79%   +<.01%     
==========================================
  Files         227      227              
  Lines       20217    20214       -3     
  Branches     3236     3236              
==========================================
  Hits        10672    10672              
+ Misses       9545     9542       -3
Flag Coverage Δ
#unittest 52.79% <0%> (ø) ⬆️
Impacted Files Coverage Δ
...pp/renderer/components/navigation/browserAction.js 30.98% <0%> (+1.25%) ⬆️

@NejcZdovc
Copy link
Contributor

I just tried it again and this fix fixes icon problem, but if you click on an icon nothing happens. But if you click on an icon when you are on new tab, popup is shown.

@jonathansampson
Copy link
Collaborator Author

@NejcZdovc This PR is meant only to resolve the missing icon issue. I'm going to look into the reason behind unresponsive icons next. Also testing the latest version of bitwarden in the process; attempting to take out as many birds with as few stones as possible :)

@bsclifton
Copy link
Member

@NejcZdovc looks like you missed 0.20.x when cherry-picking into branches. I went ahead and made sure it's included in there: e4e0cf6

@bsclifton
Copy link
Member

BTW- awesome job knocking this out, @jonathansampson 😄

@NejcZdovc
Copy link
Contributor

 @bsclifton you are right, I forgot that we are already on 0.21 😄 thank you

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants