Update: SVG workand clean up on about pref page #6904

gyandeeps · 2017-01-28T14:33:50Z

Test Plan

Go to about:preferences
Look at navigation area on left it should match the below picture (all images being SVG)
The helpful hints should be at the bottom of the navigation area
Click each option (general, search, tabs, etc) and it should have a highlight state also

Description

fixes #6931
This is cleanup work to consume svg icons on the about:pref page.

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

luixxiul · 2017-01-28T14:37:06Z

Thanks for your hard work. Would you please open an issue or associate this PR to the existing one if any so @bradleyrichter and @bsclifton would review the PR?

luixxiul · 2017-01-28T14:40:16Z

And I am afraid this work woud need refactoring to Aphrodite (to avoid duplicate work). This is the guideline that might be worth reading if you have not looked at: https://github.com/brave/browser-laptop/blob/master/docs/style.md

CC @bsclifton for an advice

bsclifton · 2017-01-30T01:16:05Z

@gyandeeps I can help you make this Aphrodite friendly 😄 I should be able to review this soon (hang in there!)

In the meantime, can you create an issue for this? I know we originally wanted to solve the URL bar icons (which is captured with #5891)

bsclifton

Per our conversation on Slack, @gyandeeps, if you could rework this using the Aphrodite styles that would be great 😄 It would basically involve creating a component and putting the styles in JavaScript. Let me know if I can help in any way. Thanks again! 😄

cezaraugusto · 2017-02-03T21:01:25Z

@gyandeeps nice work here. Based on @bsclifton request, we have a new wiki page regarding how to refactor styles which may help you:

https://github.com/brave/browser-laptop/wiki/Refactoring-styles-to-Aphrodite

cezaraugusto · 2017-02-15T21:30:18Z

Split prefs nav components, refactored to Aphrodite, rebased. Ready for code review.

Note:
Needs resolution about sync tab (disabled on this PR at this moment). /cc @bradleyrichter.

Thanks @gyandeeps for the great initial work. Way better overall styles and flexbox usage. 🎉

luixxiul · 2017-02-16T01:53:29Z

js/about/preferences.js

 const ModalOverlay = require('../components/modalOverlay')
 const {SettingsList, SettingItem, SettingCheckbox, SiteSettingCheckbox} = require('../../app/renderer/components/settings')
 const {SettingTextbox} = require('../../app/renderer/components/textbox')
 const {SettingDropdown} = require('../../app/renderer/components/dropdown')
 const Button = require('../components/button')
-const HelpfulHints = require('../../app/renderer/components/preferences/helpfulHints')


@cezaraugusto should this be deleted?

yes it migrated to preferenceNavigation

gyandeeps · 2017-02-16T03:32:57Z

@cezaraugusto Latest changes broke the change I made, where i moved the lower hint text block to always be at the bottom and on resize stick to the bottom.

Current:

https://cloud.githubusercontent.com/assets/5554486/23006222/2e7ada34-f3c6-11e6-9824-7a224fab27aa.png

Expected:

https://bravesoftware.slack.com/files/bradrichter/F3XQDKR0X/pasted_image_at_2017_01_26_01_43_pm.png

Notes:

Avoid using width and height on flexed items. (the rule i follow is to not use height and width if i use flex for the most part). That how things remain fluid and responsive.

bradleyrichter · 2017-02-16T03:44:38Z

That was a nice improvement. Need to keep that part of the PR working.

…

On Feb 15, 2017, at 7:33 PM, Gyandeep Singh ***@***.***> wrote: @cezaraugusto Latest changes broke the change I made, where i moved the lower hint text block to always be at the bottom annd on resize stick to the bottom. Current: Expected: https://bravesoftware.slack.com/files/bradrichter/F3XQDKR0X/pasted_image_at_2017_01_26_01_43_pm.png — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

cezaraugusto · 2017-02-22T21:40:22Z

app/renderer/components/preferences/helpfulHints.js

-    padding: '30px 0',
-    marginTop: '20px'
+    padding: '20px 0 0',
+    overflowY: 'auto',


adding sync and plugins tabs made menu taller, so helpfulHints got cut.

For small displays it adds a scrollbar to address that

cezaraugusto · 2017-02-22T21:57:43Z

app/renderer/components/preferences/helpfulHints.js

+    padding: '20px 0 0',
+    overflowY: 'auto',
+
+    '@media (min-height: 620px)': {


if we have enough display height then put hints at screen's bottom.

Ideally we should do it on navigation itself but then we lost the active indicator (arrow)

cezaraugusto · 2017-02-23T03:41:33Z

@gyandeeps @bradleyrichter sorry I was unaware of those changes. Updated.

@bsclifton

auditor @bsclifton fixes #6931

@cezaraugusto

Auditors: @cezaraugusto

bsclifton

Great job with the original cleanup @gyandeeps and awesome job @cezaraugusto with the Aphrodite refactor 😄 I put some stubs in place in paymentsTabTest.js and preferencesTest.js to fix broken tests. Other than that, everything looks great! 🎉

gyandeeps requested review from bsclifton and bradleyrichter January 28, 2017 14:35

luixxiul added needs-info Another team member needs information from the PR/issue opener. refactoring labels Jan 28, 2017

bsclifton suggested changes Jan 30, 2017

View reviewed changes

luixxiul added the PR/work-in-progress ⚒ label Jan 31, 2017

bsclifton added this to the 0.13.3 milestone Jan 31, 2017

gyandeeps force-pushed the pref-svg branch from a1a12b2 to 809bc75 Compare February 3, 2017 02:39

luixxiul added refactoring/aphrodite and removed needs-info Another team member needs information from the PR/issue opener. refactoring labels Feb 4, 2017

bsclifton modified the milestones: 0.13.5, 0.13.4 Feb 8, 2017

cezaraugusto self-assigned this Feb 8, 2017

luixxiul added the feature/about-pages label Feb 13, 2017

cezaraugusto mentioned this pull request Feb 14, 2017

Split and refactor to Aphrodite HelpfulHints preferences component #7232

Closed

cezaraugusto force-pushed the pref-svg branch from 809bc75 to 7e95e17 Compare February 15, 2017 21:23

cezaraugusto added PR/needs-QA-attention ☕ and removed PR/work-in-progress ⚒ labels Feb 15, 2017

luixxiul reviewed Feb 16, 2017

View reviewed changes

cezaraugusto force-pushed the pref-svg branch from 7e95e17 to 9ad0f37 Compare February 22, 2017 21:38

cezaraugusto reviewed Feb 22, 2017

View reviewed changes

gyandeeps and others added 2 commits February 23, 2017 00:36

Update: SVG workand clean up on about pref page

7c11e40

auditor @bsclifton fixes #6931

Fix unit tests

aa3757a

Auditors: @cezaraugusto

bsclifton force-pushed the pref-svg branch from 9ad0f37 to aa3757a Compare February 23, 2017 08:11

bsclifton approved these changes Feb 23, 2017

View reviewed changes

bsclifton merged commit f04dc42 into master Feb 23, 2017

bsclifton deleted the pref-svg branch February 23, 2017 08:15

bsclifton modified the milestones: 0.13.5, 0.13.6 Feb 23, 2017

bsclifton added QA/test-plan-specified release-notes/include and removed PR/needs-QA-attention ☕ labels Feb 23, 2017

alexwykoff mentioned this pull request Feb 26, 2017

Manual test template for 0.13.5 #7389

Closed

bsclifton mentioned this pull request Feb 27, 2017

convert URL bar FA icons to sprite PNG (or SVG) #5891

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update: SVG workand clean up on about pref page #6904

Update: SVG workand clean up on about pref page #6904

gyandeeps commented Jan 28, 2017 •

edited by bsclifton

Loading

luixxiul commented Jan 28, 2017

luixxiul commented Jan 28, 2017 •

edited

Loading

bsclifton commented Jan 30, 2017 •

edited

Loading

bsclifton left a comment

cezaraugusto commented Feb 3, 2017

cezaraugusto commented Feb 15, 2017 •

edited

Loading

luixxiul Feb 16, 2017

cezaraugusto Feb 16, 2017 •

edited

Loading

gyandeeps commented Feb 16, 2017 •

edited

Loading

bradleyrichter commented Feb 16, 2017 via email

cezaraugusto Feb 22, 2017

cezaraugusto Feb 22, 2017

cezaraugusto commented Feb 23, 2017

bsclifton left a comment

Update: SVG workand clean up on about pref page #6904

Update: SVG workand clean up on about pref page #6904

Conversation

gyandeeps commented Jan 28, 2017 • edited by bsclifton Loading

Test Plan

Description

luixxiul commented Jan 28, 2017

luixxiul commented Jan 28, 2017 • edited Loading

bsclifton commented Jan 30, 2017 • edited Loading

bsclifton left a comment

Choose a reason for hiding this comment

cezaraugusto commented Feb 3, 2017

cezaraugusto commented Feb 15, 2017 • edited Loading

luixxiul Feb 16, 2017

Choose a reason for hiding this comment

cezaraugusto Feb 16, 2017 • edited Loading

Choose a reason for hiding this comment

gyandeeps commented Feb 16, 2017 • edited Loading

bradleyrichter commented Feb 16, 2017 via email

cezaraugusto Feb 22, 2017

Choose a reason for hiding this comment

cezaraugusto Feb 22, 2017

Choose a reason for hiding this comment

cezaraugusto commented Feb 23, 2017

bsclifton left a comment

Choose a reason for hiding this comment

gyandeeps commented Jan 28, 2017 •

edited by bsclifton

Loading

luixxiul commented Jan 28, 2017 •

edited

Loading

bsclifton commented Jan 30, 2017 •

edited

Loading

cezaraugusto commented Feb 15, 2017 •

edited

Loading

cezaraugusto Feb 16, 2017 •

edited

Loading

gyandeeps commented Feb 16, 2017 •

edited

Loading