Add private new tab page #105
Conversation
cezaraugusto
force-pushed
the
about/newtab/private
branch
from
02b11b0
to
6142e6d
Compare
There was a problem hiding this comment.
If I'm reading this right then it looks like you're adding brave-extension as a submodule. It should instead just be automatically downloaded after the npm run init so pls remove that if it was added as a submodule here. I might be misreading the PR though?
"Submodule brave-extension added at 510031"
| { "6d23ab5117038380fd25f70909340a87.woff2", IDR_BRAVE_TEXT_FONT_MULI_800_NORMAL_LATIN_EXT }, | ||
| { "ed853eeb06da3de8325c2a4abddc30c8.woff2", IDR_BRAVE_TEXT_FONT_MULI_800_NORMAL_LATIN }, | ||
| { "7d0f2d0b966f31e8bc75bcf6954b6b49.woff2", IDR_BRAVE_TEXT_FONT_MULI_900_NORMAL_LATIN_EXT }, | ||
| { "18a1df59976e7a18fed257368aaafa9f.woff2", IDR_BRAVE_TEXT_FONT_MULI_900_NORMAL_LATIN }, |
There was a problem hiding this comment.
Could we only include the fonts that are used?
| }, | ||
| { | ||
| "name": "Cezar Augusto", | ||
| "email": "cezar@brave.com" |
| // TODO | ||
| // return <NewPrivateTab newTabData={newTabData} /> | ||
| return null | ||
| return <NewPrivateTab stats={newTabData.stats} /> |
There was a problem hiding this comment.
Height needs to be adjusted here so it takes up the whole height for the background.
| } | ||
| } | ||
|
|
||
| module.exports = Clock |
There was a problem hiding this comment.
Nice to see this removed from here! 👍
| fontFamily: '"Poppins", sans-serif', | ||
| color: 'rgba(255,255,255,0.8)', | ||
| padding: '80px 60px 40px', | ||
| background: 'linear-gradient(#4b3c6e, #000)' |
There was a problem hiding this comment.
Should we have some of this factored out from the inline component structure?
| <SwitchButton | ||
| id='togglePrivateSearchEngine' | ||
| size='large' | ||
| checked={false /* TODO: enable search engine change */} |
There was a problem hiding this comment.
Let's put a dummy handler for now that calls to the reducer. Currently when you click the switch:
| counter={timeSaved.value} | ||
| text={window.loadTimeData.getString(timeSaved.id)} | ||
| description={window.loadTimeData.getString('estimatedTimeSaved')} /> | ||
| </DataBlock> |
There was a problem hiding this comment.
Ditto some of these inline values, wondering if we should have a separate parameters js file or have the styles in css.
cezaraugusto
force-pushed
the
about/newtab/private
branch
2 times, most recently
from
3c82f5c
to
3a1affa
Compare
|
ya, re #105 (review) seems like after an incremental build those files are added momentarily. Not sure what happened but removed from git. Other feedback addressed too, thanks! |
cezaraugusto
force-pushed
the
about/newtab/private
branch
2 times, most recently
from
5f0076b
to
b277e14
Compare
There was a problem hiding this comment.
I'm ok with the changes but I think there's problem with some of the included resources: No such file or directory: u'../../out/Debug/gen/brave/1559d981ccec8dc4b4feb9043271ab3d.woff2'
Also I think there are more .woff2 files added than defined in the css and in the grd.
cezaraugusto
force-pushed
the
about/newtab/private
branch
3 times, most recently
from
c8f2b81
to
91e1044
Compare
bbondy
force-pushed
the
about/newtab/private
branch
from
91e1044
to
4a6671d
Compare
cezaraugusto
force-pushed
the
about/newtab/private
branch
from
4a6671d
to
91e1044
Compare
- includes images, fonts and locale strings used in private newtab page
- this avoid the require of 6 fonts for newtab page
bbondy
force-pushed
the
about/newtab/private
branch
from
91e1044
to
9fe0aa3
Compare
|
Nice work! |
closes brave/brave-browser#200
Note: private search engine toggle is only aesthetic at this moment.