Add support for welcome page #8097

cezaraugusto · 2017-04-06T03:45:36Z

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).

Test Plan:

npm run test -- --grep="AboutWelcome component"

QA steps:

Navigate to about:welcome
should load content from https://brave.com/welcome.html

bsclifton · 2017-04-09T19:28:56Z

Moving to 0.14.3

cezaraugusto · 2017-04-10T08:13:00Z

just as a heads-up, blocker for PR fixed by https://github.com/brave/brave-website/commit/774bf249ebc3d356ae52f2d18d6e106cd7d55cf2. Needs landing on master for this being approved.

cezaraugusto · 2017-04-18T03:07:40Z

test/unit/about/welcomePageTest.js

@@ -0,0 +1,35 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public


changed to Enzyme instead of Webdriver

cezaraugusto · 2017-04-18T03:09:59Z

app/extensions/brave/content/styles/defaultStyles.css

@@ -0,0 +1,18 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public


future refactoring of about pages will benefit from this instead of relying on common.less file so I took advantage of this PR to add it since it's needed anyway for welcome page

bsclifton

Code changes LGTM 😄

I wasn't able to get it working locally though, because master seems to be having an issue causing a crash. I know there's still more work to be done- ping me and I'll follow up 😄

cezaraugusto · 2017-05-08T19:51:06Z

moving to 0.15.4

srirambv · 2017-05-09T07:27:56Z

@cezaraugusto this shows double vertical scroll bar on about:welcome. Is that expected??

cezaraugusto · 2017-05-10T00:07:24Z

thanks @srirambv, fixed. the joy of having it hidden by default :)

bsclifton · 2017-05-17T17:36:21Z

@cezaraugusto @bradleyrichter this is ready for review, right? 😄

cezaraugusto · 2017-05-17T18:31:42Z

@bradleyrichter just pushed website to prod so yes, ready

luixxiul · 2017-05-18T02:18:38Z

test/unit/about/welcomePageTest.js

+let AboutWelcome
+require('../braveUnit')
+
+describe('AboutWelcome component', function () {


Is this intentional? The test plan says npm run test -- --grep="about:welcome", which does not trigger that.

I just updated it 😄

thanks し( ・∀・)／＼(・∀・) ///

luixxiul

I think it would be better if the import dialog is directly displayed when you click I'm ready button.

luixxiul · 2017-05-18T02:23:34Z

bradleyrichter · 2017-05-18T02:53:50Z

Yes it would. But we have exhausted ideas for this version unless you have a new idea?

…

On May 17, 2017, at 7:23 PM, Suguru Hirahara ***@***.***> wrote: <https://cloud.githubusercontent.com/assets/3362943/26183846/635a05ca-3bbc-11e7-881a-d13bf3f9d12a.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#8097 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AM4jqv10V2DjvwIQiBk3Gq8rxplS15JXks5r66usgaJpZM4M1FXw>.

luixxiul · 2017-05-18T02:58:07Z

unless you have a new idea?

how about moving import button on about:preferences#general up so that it would get displayed without scrolling? On MBP you have to scroll to find out the button.

bsclifton · 2017-05-18T07:03:54Z

We could create a deep link using the URL, like about:preferences#general?import. It would take a little work to get that to parse out and then interpreted in the React component, but it's possible. We should be able to make that URL pop up the modal

NejcZdovc · 2017-05-18T07:06:43Z

@bsclifton we need to do this anyway for ledger as well. I would suggest that we create a separate issue and do it in 0.15.400 (ledger needs it in this version).

bsclifton · 2017-05-18T07:20:39Z

@NejcZdovc good call- can you create the issue (capturing this use case and also the one for ledger)

bsclifton · 2017-05-19T18:57:11Z

@cezaraugusto @bradleyrichter is this screen supposed to show on first launch? When I have a new session, it still just loads a new tab with the dashboard (and of course, I didn't see any code changes for this functionality)

We could always make a new issue like "load about:welcome on first launch" if we wanted to follow up this PR with the fix

bsclifton · 2017-05-19T19:02:42Z

Since there's an iframe inside the page, it's showing a scrollbar which looks kind of weird:

The iframe should have the scrollbar (this makes sense), but the page itself shouldn't. This is happening in part because the height is set as 100% (for a quick demo, open dev tools and set to 99%)

bsclifton · 2017-05-19T19:11:36Z

@NejcZdovc I created the issue for the deep link functionality 😄
#8966

@bsclifton

- Auditors: @bsclifton - Closes #7821

cezaraugusto · 2017-05-22T19:24:11Z

ok rebased & updated

cezaraugusto · 2017-05-22T19:24:33Z

app/extensions/brave/content/styles/defaultStyles.css

+}
+
+.welcomePage {
+  overflow: hidden;


here's our hero for double scrollbars

luixxiul

Both manual and automated test pass. LGTM.

cezaraugusto added the feature/about-pages label Apr 6, 2017

cezaraugusto added this to the 0.14.2 milestone Apr 6, 2017

cezaraugusto self-assigned this Apr 6, 2017

cezaraugusto requested a review from bsclifton April 6, 2017 03:45

cezaraugusto added the PR/blocked label Apr 6, 2017

bsclifton modified the milestones: 0.14.3, 0.14.2 Apr 9, 2017

cezaraugusto commented Apr 18, 2017

View reviewed changes

bsclifton reviewed Apr 22, 2017

View reviewed changes

bbondy force-pushed the master branch from f0df59d to 70e5d77 Compare April 23, 2017 04:21

alexwykoff modified the milestones: 0.15.4, 0.15.3 May 5, 2017

cezaraugusto modified the milestones: 0.15.3, 0.15.4 May 5, 2017

cezaraugusto modified the milestones: 0.15.4, 0.15.3 May 8, 2017

cezaraugusto added PR/needs-QA-attention ☕ and removed PR/blocked labels May 17, 2017

cezaraugusto requested a review from luixxiul May 17, 2017 19:20

luixxiul reviewed May 18, 2017

View reviewed changes

bsclifton mentioned this pull request May 19, 2017

Support a deep link URL structure for Brave pages #8966

Open

Add support for welcome page

f1db674

- Auditors: @bsclifton - Closes #7821

cezaraugusto commented May 22, 2017

View reviewed changes

luixxiul approved these changes May 23, 2017

View reviewed changes

luixxiul merged commit 123f609 into brave:master May 26, 2017

luixxiul removed the PR/needs-QA-attention ☕ label May 26, 2017

bsclifton mentioned this pull request May 26, 2017

Integrate the new welcome screen #7821

Closed

cezaraugusto deleted the about/welcome branch July 25, 2017 07:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for welcome page #8097

Add support for welcome page #8097

cezaraugusto commented Apr 6, 2017 •

edited by bsclifton

Loading

bsclifton commented Apr 9, 2017

cezaraugusto commented Apr 10, 2017

cezaraugusto Apr 18, 2017

cezaraugusto Apr 18, 2017 •

edited

Loading

bsclifton left a comment

cezaraugusto commented May 8, 2017

srirambv commented May 9, 2017

cezaraugusto commented May 10, 2017

bsclifton commented May 17, 2017

cezaraugusto commented May 17, 2017

luixxiul May 18, 2017

bsclifton May 19, 2017

cezaraugusto May 19, 2017

luixxiul left a comment

luixxiul commented May 18, 2017

bradleyrichter commented May 18, 2017 via email

luixxiul commented May 18, 2017

bsclifton commented May 18, 2017 •

edited

Loading

NejcZdovc commented May 18, 2017 •

edited

Loading

bsclifton commented May 18, 2017

bsclifton commented May 19, 2017 •

edited

Loading

bsclifton commented May 19, 2017 •

edited

Loading

bsclifton commented May 19, 2017

cezaraugusto commented May 22, 2017

cezaraugusto May 22, 2017

luixxiul left a comment

		@@ -0,0 +1,35 @@
		/* This Source Code Form is subject to the terms of the Mozilla Public

		@@ -0,0 +1,18 @@
		/* This Source Code Form is subject to the terms of the Mozilla Public

Add support for welcome page #8097

Add support for welcome page #8097

Conversation

cezaraugusto commented Apr 6, 2017 • edited by bsclifton Loading

bsclifton commented Apr 9, 2017

cezaraugusto commented Apr 10, 2017

cezaraugusto Apr 18, 2017

Choose a reason for hiding this comment

cezaraugusto Apr 18, 2017 • edited Loading

Choose a reason for hiding this comment

bsclifton left a comment

Choose a reason for hiding this comment

cezaraugusto commented May 8, 2017

srirambv commented May 9, 2017

cezaraugusto commented May 10, 2017

bsclifton commented May 17, 2017

cezaraugusto commented May 17, 2017

luixxiul May 18, 2017

Choose a reason for hiding this comment

bsclifton May 19, 2017

Choose a reason for hiding this comment

cezaraugusto May 19, 2017

Choose a reason for hiding this comment

luixxiul left a comment

Choose a reason for hiding this comment

luixxiul commented May 18, 2017

bradleyrichter commented May 18, 2017 via email

luixxiul commented May 18, 2017

bsclifton commented May 18, 2017 • edited Loading

NejcZdovc commented May 18, 2017 • edited Loading

bsclifton commented May 18, 2017

bsclifton commented May 19, 2017 • edited Loading

bsclifton commented May 19, 2017 • edited Loading

bsclifton commented May 19, 2017

cezaraugusto commented May 22, 2017

cezaraugusto May 22, 2017

Choose a reason for hiding this comment

luixxiul left a comment

Choose a reason for hiding this comment

cezaraugusto commented Apr 6, 2017 •

edited by bsclifton

Loading

cezaraugusto Apr 18, 2017 •

edited

Loading

bsclifton commented May 18, 2017 •

edited

Loading

NejcZdovc commented May 18, 2017 •

edited

Loading

bsclifton commented May 19, 2017 •

edited

Loading

bsclifton commented May 19, 2017 •

edited

Loading