Fixed issue #13256 - Removing or modifying URL will now hide EVC #13295

jasonrsadler · 2018-02-24T15:42:59Z

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.
Request a security/privacy review as needed. (Ask a Brave employee to help if you cannot access this document.)

Test Plan:

Fixes: #13256

Reviewer Checklist:

Request a security/privacy review as needed if one was not already requested.

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

@bsclifton @srirambv

codecov-io · 2018-02-24T16:00:10Z

Codecov Report

Merging #13295 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master   #13295   +/-   ##
=======================================
  Coverage   56.01%   56.01%           
=======================================
  Files         282      282           
  Lines       28211    28211           
  Branches     4642     4642           
=======================================
  Hits        15801    15801           
  Misses      12410    12410

Flag	Coverage Δ
#unittest	`56.01% <0%> (ø)`	⬆️

Impacted Files	Coverage Δ
...r/components/preferences/payment/ledgerRecovery.js	`37.25% <0%> (ø)`	⬆️

alexwykoff · 2018-02-27T18:17:40Z

If not reviewed by eob, will be punted to later release.

diracdeltas · 2018-02-27T18:36:23Z

app/renderer/components/navigation/urlBar.js

@@ -64,7 +65,7 @@ class UrlBar extends React.Component {
  }

  maybeUrlBarTextChanged (value) {
-    if (value !== this.props.urlbarLocation) {
+    if (value !== this.props.urlbarLocation) {      


please remove trailing whitespace. i think npm run lint should catch this

diracdeltas · 2018-02-27T18:37:03Z

app/renderer/components/navigation/urlBar.js

@@ -52,6 +52,7 @@ class UrlBar extends React.Component {
    this.lastVal = ''
    this.lastSuffix = ''
    this.isOnComposition = false
+    this.isChanging = false;


no trailing semicolon

jasonrsadler · 2018-02-27T18:52:56Z

Running lint did indeed catch it. Sorry about those old habits.

diracdeltas · 2018-02-27T18:54:11Z

app/renderer/components/navigation/urlBar.js

@@ -501,7 +513,7 @@ class UrlBar extends React.Component {
      <UrlBarIcon
        titleMode={this.props.titleMode}
      />
-      {this.showEvCert}
+      {this.showEvCert }


extra space before the bracket

Linter didn't catch that one

diracdeltas · 2018-02-27T18:58:21Z

app/renderer/components/navigation/urlBar.js

@@ -52,6 +52,7 @@ class UrlBar extends React.Component {
    this.lastVal = ''
    this.lastSuffix = ''
    this.isOnComposition = false
+    this.isChanging = false


why not use this.props.isActive for this purpose?

jasonrsadler · 2018-02-27T19:24:05Z

Yes. That works. Didn't know about props.isActive.

Fixed issue #13256 - Removing or modifying URL will now hide EVC Updated to pass linting Convention clean up Fixed issue #13256 - Removing or modifying URL will now hide EVC

# This is the 1st commit message: Fixes #11422, keeps recovery modal open after a recovery failure # This is the commit message #2: Fixed issue #13256 - Removing or modifying URL will now hide EVC # This is the commit message #3: Updated to pass linting # This is the commit message #4: Convention clean up

diracdeltas · 2018-02-27T19:37:45Z

app/renderer/components/navigation/urlBar.js

@@ -489,7 +493,7 @@ class UrlBar extends React.Component {
  }

  get showEvCert () {
-    if (this.props.titleMode) {
+    if (this.props.titleMode || this.props.isActive) {


awesome, this simplifies things a lot :)

diracdeltas · 2018-02-27T19:38:21Z

app/renderer/components/preferences/payment/ledgerRecovery.js

@@ -31,8 +31,10 @@ class LedgerRecoveryContent extends ImmutableComponent {
    this.props.handleRecoveryKeyChange(e.target.value)
  }

-  clearRecoveryStatus () {
-    this.props.hideAdvancedOverlays()
+  clearRecoveryStatus (success) {


i think the changes to this and the following files aren't supposed to be here

I went back too far on the squash

@diracdeltas Is this back to normal with the right changes?

jasonrsadler · 2018-02-27T20:28:08Z

I hosed this. Moving to PR #13335

srirambv requested review from cezaraugusto, bsclifton and NejcZdovc February 27, 2018 02:06

srirambv assigned jasonrsadler Feb 27, 2018

srirambv added feature/URLbar PR/pending-review labels Feb 27, 2018

srirambv added this to the 0.23.x (Nightly Channel) milestone Feb 27, 2018

bsclifton modified the milestones: 0.23.x (Nightly Channel), 0.24.x (coming soon), 0.22.x (Developer Channel) Feb 27, 2018

NejcZdovc removed their request for review February 27, 2018 06:06

alexwykoff added the priority/P3 Major loss of function. label Feb 27, 2018

alexwykoff modified the milestones: 0.22.x (Developer Channel), Backlog (Prioritized) Feb 27, 2018

diracdeltas self-requested a review February 27, 2018 18:32

diracdeltas reviewed Feb 27, 2018

View reviewed changes

ryanml added 2 commits February 27, 2018 14:33

Fixes #11422, keeps recovery modal open after a recovery failure

b202a3f

Fixed issue #13256 - Removing or modifying URL will now hide EVC Updated to pass linting Convention clean up Fixed issue #13256 - Removing or modifying URL will now hide EVC

diracdeltas reviewed Feb 27, 2018

View reviewed changes

Fixed issue #13256 - Removing or modifying URL will now hide EVC

92675bf

bsclifton closed this Feb 27, 2018

bsclifton removed this from the 0.22.x (Developer Channel) milestone Feb 27, 2018

bsclifton removed the PR/pending-review label Mar 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed issue #13256 - Removing or modifying URL will now hide EVC #13295

Fixed issue #13256 - Removing or modifying URL will now hide EVC #13295

jasonrsadler commented Feb 24, 2018 •

edited by srirambv

Loading

codecov-io commented Feb 24, 2018 •

edited

Loading

alexwykoff commented Feb 27, 2018

diracdeltas Feb 27, 2018

diracdeltas Feb 27, 2018

jasonrsadler commented Feb 27, 2018

diracdeltas Feb 27, 2018

jasonrsadler Feb 27, 2018

diracdeltas Feb 27, 2018

jasonrsadler commented Feb 27, 2018

diracdeltas Feb 27, 2018

diracdeltas Feb 27, 2018

jasonrsadler Feb 27, 2018 •

edited

Loading

jasonrsadler Feb 27, 2018

jasonrsadler commented Feb 27, 2018

Fixed issue #13256 - Removing or modifying URL will now hide EVC #13295

Fixed issue #13256 - Removing or modifying URL will now hide EVC #13295

Conversation

jasonrsadler commented Feb 24, 2018 • edited by srirambv Loading

Submitter Checklist:

Test Plan:

Reviewer Checklist:

codecov-io commented Feb 24, 2018 • edited Loading

Codecov Report

alexwykoff commented Feb 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonrsadler commented Feb 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonrsadler commented Feb 27, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonrsadler Feb 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonrsadler commented Feb 27, 2018

jasonrsadler commented Feb 24, 2018 •

edited by srirambv

Loading

codecov-io commented Feb 24, 2018 •

edited

Loading

jasonrsadler Feb 27, 2018 •

edited

Loading