Conversation
1023398 to
e2d086f
Compare
|
This looks great! Small things:
|
|
I made the threebox modal consistent with other modals while updating the wording to restore:
All of these modals have different buttons from those that you shared a screen shot of above. If the design system specifies a change to these modals design, we can do them all at once in a separate issue. |
15514dd to
0d1be18
Compare
|
cool, filed here, can handle later. where did the
|
8d8b58b to
9ec437d
Compare
0d1be18 to
7f13792
Compare
Cut with 8363564 I my first job a senior engineer told me that copy+paste is the devil. Its times like these I believe it... |
8363564 to
4cc7c4a
Compare
ui/app/components/app/modals/threebox-restore-confirm/threebox-restore-confirm.component.js
Outdated
Show resolved
Hide resolved
ui/app/components/app/modals/threebox-restore-confirm/threebox-restore-confirm.component.js
Outdated
Show resolved
Hide resolved
ui/app/store/actions.js
Outdated
There was a problem hiding this comment.
It looks like this was added accidentally.
There was a problem hiding this comment.
Oh wait... nevermind. This is needed - it was just missing. That's concerning 😕
There was a problem hiding this comment.
In fact, it is not needed. The case where it appears in the code is never called. I've removed the addition of this via a rebase, and will remove the case where it is called in a separate PR.
There was a problem hiding this comment.
It looks like this is still here - did it get re-added?
There was a problem hiding this comment.
It looks like this should be here - it's just broken on develop right now 😕
...first-time-flow/create-password/import-with-seed-phrase/import-with-seed-phrase.component.js
Outdated
Show resolved
Hide resolved
…se and setRestoredFromThreeBoxToTrue
…nd bg-libs in hard coded bundle list
5e641f1 to
035eb23
Compare
Builds ready [035eb23]
|
gulpfile.js
Outdated
| ...json.background, | ||
| scripts: json.background.scripts.filter(scriptName => !scriptsToExcludeFromBackgroundDevBuild[scriptName]), | ||
| } | ||
| json.permissions = [...json.permissions, 'webRequestBlocking', 'http://localhost/8889'] |
There was a problem hiding this comment.
Thoughts on whether we should change this to localhost/*? I remain confused about why localhost/8999 works in the first place. If you feel inclined to leave it as localhost/8999 we should at least make sure it works on Firefox, as I can't see any documentation to explain why it does work, so it could be some sort of fluke.
ui/app/pages/home/home.component.js
Outdated
There was a problem hiding this comment.
Conditions like this might leave us in a strange state if the flag is turned on, then off again.
This situation wouldn't occur unless someone enabled it in the dev console of course, but do you think it's worth adding the flag here to prevent it anyway? It's possible an end-user might enable 3Box in the dev console then disable it, and get in a weird state like this. Maybe unlikely though - I'm OK with ignoring this case if you'd prefer. If you did want to handle it, there are a few other examples around like this as well.
|
Also it'd be nice to have tests for the migration. I think all of the recent migrations have been tested, so it should be pretty easy to add using a previous one as a template. |
…d via dev console
035eb23 to
3d81850
Compare
|
@Gudahtt I realized we don't actually need a migration. The feature flag will be |
Builds ready [3d81850]
|
…evelop * origin/develop: (31 commits) Performance: Delivery optimized images (#7176) Add `appName` message to each locale Remove the disk store (#7170) Update @hapi/subtext as per security advisory (#7172) Add fixes for German translations (#7168) Fix recipient field of approve screen (#7171) 3box integration 2.0 (#6972) ci - metamaskbot - include links to dep-viz and all artifacts (#7155) Replace `undefined` selectedAddress with `null` (#7161) Add polyfill for AbortController (#7157) Remove redundant error logging (#7158) Set minimum Firefox version to v56.2 to support Waterfox (#7156) ci - install deps with "--har" flag to capture network activity (#7143) ci - create source-map-explorer build-artifacts (#7141) ci - build-artifacts - generate sesify-viz for inspecting deps (#7151) Publish GitHub release from master branch (#7136) fix rinkeby spelling (#7148) deps - move gulp-terser-js to devDeps test:integration - fix renamed test data file lint fix ...
This PR adds on to the current 3box-integration PR. Changes include:
e2e tests are in development, but will be added in a future PR
Demo here: https://streamable.com/ovvbf