Adds transition overlay #11395

NejcZdovc · 2017-10-09T09:57:10Z

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.

Resolves #11389
Resolves #11340
Resolves #11333
Resolves #11338

Auditors:

Test Plan:

Create wallet with 0.18 version
With this profile switch to version 0.19
You should see transition overlay with additional settings disabled
After transition is done, overlay should hide and additional settings should be re-enabled

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

luixxiul · 2017-10-09T10:07:09Z

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

+    animationName: [loaderAnimation],
+    animationDuration: '.6s',
+    animationDelay: '.1s',
+    animationIterationCount: 'infinite'


please don't forget to dedupe the animation properties later

loader__line: { display: 'inline-block', width: '15px', height: '15px', borderRadius: '15px', animationName: [loaderAnimation], animationDuration: '.6s', animationIterationCount: 'infinite' }, loader__line_1: { backgroundColor: '#FF5000', animationDelay: '.1s' }

and so on

luixxiul · 2017-10-09T10:10:55Z

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

+    const inTranstion = !(ledgerData.getIn(['migration', 'btc2BatTransitionDone']))
+
+    return <section className={css(styles.enabledContent)}>
+      <div className={css(styles.loader, inTranstion && styles.loader_show)}>


styles.enabledContent__loader, inTranstion && styles.enabledContent__loader_show. please change elements' className accordingly.

luixxiul · 2017-10-09T10:12:42Z

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

+            <div className={css(styles.loader__line, styles.loader__line_3)} />
+          </div>
+        </div>
+      </div>
      <div className={css(styles.walletBar)} data-test-id='walletBar'>


styles.walletBar -> styles.enabledContent__walletBar

this is not part of this PR

This change requires that: https://github.com/brave/browser-laptop/pull/11395/files#diff-5d8199641ea1ba12e0394859f7eb0c55R175

codecov-io · 2017-10-09T10:21:20Z

Codecov Report

Merging #11395 into master will increase coverage by 0.03%.
The diff coverage is 88.88%.

@@            Coverage Diff             @@
##           master   #11395      +/-   ##
==========================================
+ Coverage   52.52%   52.55%   +0.03%     
==========================================
  Files         267      267              
  Lines       24994    24987       -7     
  Branches     3995     4001       +6     
==========================================
+ Hits        13127    13133       +6     
+ Misses      11867    11854      -13

Flag	Coverage Δ
#unittest	`52.55% <88.88%> (+0.03%)`	⬆️

Impacted Files	Coverage Δ
app/browser/tabs.js	`26.64% <0%> (+0.55%)`	⬆️
app/browser/reducers/ledgerReducer.js	`47.05% <100%> (+0.36%)`	⬆️
app/renderer/components/preferences/paymentsTab.js	`76.92% <100%> (+2.37%)`	⬆️
app/renderer/components/styles/animations.js	`100% <100%> (ø)`	⬆️
app/browser/api/ledger.js	`30.78% <100%> (+0.18%)`	⬆️
...r/components/preferences/payment/enabledContent.js	`91.24% <91.66%> (+0.54%)`	⬆️
js/stores/appStoreRenderer.js	`91.17% <0%> (-8.83%)`	⬇️
app/renderer/components/reduxComponent.js	`84.37% <0%> (-6.25%)`	⬇️
js/stores/windowStore.js	`27.27% <0%> (-0.31%)`	⬇️
... and 5 more

cezaraugusto · 2017-10-09T15:08:18Z

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

@@ -274,6 +289,18 @@ const gridStyles = StyleSheet.create({
  }
 })

+const loaderAnimation = {


I know still WIP but please move this to animations.js file which is avail in 0.19.x too

luixxiul · 2017-10-10T04:16:05Z

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

+            <div className={css(styles.loader__line, styles.loader__line_3, !inTranstion && styles.loader__line_off)} />
+          </div>
+        </div>
+      </div>
      <div className={css(styles.walletBar)} data-test-id='walletBar'>


styles.walletBar -> styles.enabledContent__walletBar. See L175.

luixxiul · 2017-10-10T04:20:07Z

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

+          </div>
+        </div>
+      </div>
+      <div className={css(styles.enabledContent__walletBar)} data-test-id='walletBar'>
        <div className={css(gridStyles.row1col1, styles.walletBar__title)} data-l10n-id='monthlyBudget' />


enabledConent__walletBar__title. please change every element under enabledContent__walletBar, thanks. See L175.

Sweet thanks!

bsclifton · 2017-10-10T18:42:33Z

app/browser/tabs.js

@@ -198,6 +199,7 @@ const updateAboutDetails = (tab, tabValue) => {
      .merge(synopsis)
      .merge(preferencesData)
      .set('wizardData', wizardData)
+      .set('migration', migration)


Love this! Great use of the new migration element

@bsclifton

Resolves brave#11389 Resolves brave#11340 Resolves brave#11333 Resolves brave#11338 Auditors: @bsclifton Test Plan:

bsclifton

LGTM! I ran through the process and verified each of the issues was covered. I fixed some variable misspellings while reviewing the code. Great job 😄 👍

Adds transition overlay

bsclifton · 2017-10-10T21:33:48Z

master 1d4edf4
0.20.x 371ccf5
0.19.x bb8be0d

@NejcZdovc since there were conflicts, would you mind checking my conflict resolution? I manually tested the following on each:

use fresh profile with 0.18.x branch
create bitcoin wallet
update branch
run and confirm transition shows

NejcZdovc added feature/rewards PR/work-in-progress ⚒ labels Oct 9, 2017

NejcZdovc self-assigned this Oct 9, 2017

NejcZdovc added this to the 0.19.x (Beta Channel) milestone Oct 9, 2017

luixxiul reviewed Oct 9, 2017

View reviewed changes

cezaraugusto reviewed Oct 9, 2017

View reviewed changes

NejcZdovc force-pushed the hotfix/#11389-overlay branch 2 times, most recently from f39a18a to 624d39c Compare October 10, 2017 03:39

NejcZdovc removed the PR/work-in-progress ⚒ label Oct 10, 2017

NejcZdovc requested review from luixxiul, cezaraugusto and bsclifton October 10, 2017 04:10

NejcZdovc force-pushed the hotfix/#11389-overlay branch 2 times, most recently from eae3582 to d9bd125 Compare October 10, 2017 04:11

luixxiul reviewed Oct 10, 2017

View reviewed changes

NejcZdovc force-pushed the hotfix/#11389-overlay branch from d9bd125 to 44800cd Compare October 10, 2017 04:18

luixxiul reviewed Oct 10, 2017

View reviewed changes

NejcZdovc force-pushed the hotfix/#11389-overlay branch 3 times, most recently from 58b402a to 3e7bd2f Compare October 10, 2017 04:28

NejcZdovc mentioned this pull request Oct 10, 2017

Progress screen while in transition #11389

Closed

bsclifton mentioned this pull request Oct 10, 2017

Notification not shown after transition was triggered #11333

Closed

This was referenced Oct 10, 2017

Disable backup wallet button until wallet is ready #11340

Closed

Currency QR Code missing after upgrade #11338

Closed

bsclifton reviewed Oct 10, 2017

View reviewed changes

Adds transition overlay

c7cbee1

Resolves brave#11389 Resolves brave#11340 Resolves brave#11333 Resolves brave#11338 Auditors: @bsclifton Test Plan:

bsclifton force-pushed the hotfix/#11389-overlay branch from 3e7bd2f to c7cbee1 Compare October 10, 2017 18:44

bsclifton approved these changes Oct 10, 2017

View reviewed changes

bsclifton merged commit 1d4edf4 into brave:master Oct 10, 2017

bsclifton added a commit that referenced this pull request Oct 10, 2017

Merge pull request #11395 from NejcZdovc/hotfix/#11389-overlay

371ccf5

Adds transition overlay

bsclifton added a commit that referenced this pull request Oct 10, 2017

Merge pull request #11395 from NejcZdovc/hotfix/#11389-overlay

bb8be0d

Adds transition overlay

This was referenced Oct 14, 2017

Warning: validateDOMNesting(...) on enabledContent.js #11523

Closed

Fix "Warning: validateDOMNesting(...)" error #11524

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds transition overlay #11395

Adds transition overlay #11395

NejcZdovc commented Oct 9, 2017 •

edited

Loading

luixxiul Oct 9, 2017

NejcZdovc Oct 10, 2017

luixxiul Oct 10, 2017

NejcZdovc Oct 10, 2017

luixxiul Oct 9, 2017

NejcZdovc Oct 10, 2017

luixxiul Oct 9, 2017

NejcZdovc Oct 10, 2017

luixxiul Oct 10, 2017

codecov-io commented Oct 9, 2017 •

edited

Loading

cezaraugusto Oct 9, 2017

NejcZdovc Oct 10, 2017

luixxiul Oct 10, 2017

NejcZdovc Oct 10, 2017

luixxiul Oct 10, 2017 •

edited

Loading

NejcZdovc Oct 10, 2017

luixxiul Oct 10, 2017

bsclifton Oct 10, 2017

bsclifton left a comment

bsclifton commented Oct 10, 2017

Adds transition overlay #11395

Adds transition overlay #11395

Conversation

NejcZdovc commented Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Oct 9, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

luixxiul Oct 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsclifton left a comment

Choose a reason for hiding this comment

bsclifton commented Oct 10, 2017

NejcZdovc commented Oct 9, 2017 •

edited

Loading

codecov-io commented Oct 9, 2017 •

edited

Loading

luixxiul Oct 10, 2017 •

edited

Loading