WIP: Style updates to Settings #623
Conversation
bsclifton
force-pushed
the
bsc-prefs-sidebar
branch
from
4e58adc to
90ad0dc
Compare
|
Did some light CSS and icon render fixes. Latest screen: |
bsclifton
force-pushed
the
bsc-prefs-sidebar
branch
5 times, most recently
from
af43ddd to
f16a26a
Compare
|
Just finished rebasing this and making sure it works great against master 😄 👍 |
| <dom-module id="settings-icons"> | ||
| <template> | ||
| <style> | ||
| -<if expr="chromeos"> |
There was a problem hiding this comment.
I'm confused about this patch. This only applies to chromeos
There was a problem hiding this comment.
looks like just an indentation thing? I can fix it
| <template> | ||
| <style include="settings-shared"> | ||
| :host { | ||
| - display: block; |
There was a problem hiding this comment.
we need to look at other ways to do this, this patch is going to be a nightmare to maintain
|
we definitely need to look at other ways to handle these patches, they are going to be effectively unmaintainable across chromium upgrades |
| <div id="leftSpacer"> | ||
| <!-- Note: showing #menuPromo relies on this dom-if being [restamp]. --> | ||
| - <template is="dom-if" if="[[showMenu]]" restamp> | ||
| + <!-- <template is="dom-if" if="[[showMenu]]" restamp> |
There was a problem hiding this comment.
if this is based on showMenu then why don't we just turn off showMenu?
There was a problem hiding this comment.
showMenu is a function on the prototype for MenuButton, and not a property / variable. So I think we'll have to extend and set to undefined.
|
|
||
| AboutHandler* AboutHandler::Create(content::WebUIDataSource* html_source, | ||
| Profile* profile) { | ||
| + std::string brave_product_version = version_info::GetVersionNumber(); |
There was a problem hiding this comment.
I believe this logic already exists somewhere and if it doesn't it needs to be put in a helper. It definitely doesn't belong here in this patch
Making good progress :) - `Settings` header added over primary items (and restyled `Advanced` to match) - Padding and margin appear to match spec - Specified different font; page needs to be wired to properly include it - Sidebar has white background + borders with radius - Colors: - active items use `brandBrave` orange and bold the text - header incidently is now orange also - inactive items use `subtleInteracting` - `About Brave` text uses `subtle` - Product version (ex: 0.55.2) shown under `About Brave` along with a default icon - `Extensions` moved up under `Settings` - `Appearance` moved to last item before `Advanced`
- fixed render issues with current icons - did some light css but looped in pete and found it might be diminishing return so stopped - made the top banner a gradient - rebased against master 2019 Jan 10
bsclifton
force-pushed
the
bsc-prefs-sidebar
branch
from
f16a26a to
9aa3f7c
Compare
| @@ -0,0 +1,36 @@ | |||
| <link rel="import" href="../html/polymer.html"> | |||
There was a problem hiding this comment.
Leaving a note here to remember to change this path to point at the path in chromium's resources
|
Really looking forward to seeing this merged. This looks really promising 👍 |
petemill
force-pushed
the
bsc-prefs-sidebar
branch
2 times, most recently
from
4cd31c8 to
a4f1022
Compare
|
Closing in favor of the branch @petemill has been working on 😄 |
Current status
This is something I've worked on as time allows. Feel free to grab it and help! Feedback appreciated
This PR is meant to fix:
brave/brave-browser#958
brave/brave-browser#964
And partially addresses:
brave/brave-browser#1222
brave/brave-browser#1186
brave/brave-browser#960
brave/brave-browser#955
brave/brave-browser#26
(and possibly others)
Here's a screenshot showing how it looks at the moment:
TODOs
chrome://settings/shields)Get Startedsection and condense appropriate settings thereSubmitter Checklist:
npm test brave_unit_tests && npm test brave_browser_tests) ongit rebase master(if needed).git rebase -ito squash commits (if needed).Test Plan:
Reviewer Checklist: