chrome/browser/resources/cleanup_tool/cleanup.html - Issue 2784653004: Cleanup Tool WebUI: Add HTML content and styling with dummy JS

Side by Side Diff: chrome/browser/resources/cleanup_tool/cleanup.html

Issue 2784653004: Cleanup Tool WebUI: Add HTML content and styling with dummy JS (Closed)

Patch Set: Add missing dtor Created 3 years, 8 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

OLD	NEW
(Empty)
	1 <!doctype html>

	2 <html dir="$i18n{textdirection}" lang="$i18n{language}">

	3 <head>

	4 <meta name="google" value="notranslate">

	5 <meta charset="utf-8">

	6 <title>$i18n{title}</title>

	7 <link rel="stylesheet" href="chrome://resources/css/text_defaults_md.css">

	8 <link rel="stylesheet" href="chrome://resources/css/md_colors.css">

	9

	10 <style>

	11 html {

	12 background: var(--md-background-color);

	13 }

	14

	15 body {

	16 margin: 0;

	17 }

	18

	19 :root {
	Dan Beam 2017/04/04 17:40:57 fwiw: :root and html are probably equivalent here fwiw: :root and html are probably equivalent here proberge 2017/04/04 19:08:33 There's no uses of --my_css_rule: foo within html Show quoted text On 2017/04/04 17:40:57, Dan Beam (slow) wrote: > fwiw: :root and html are probably equivalent here There's no uses of --my_css_rule: foo within html or body style tags, and a few uses within root: https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+html%5Cs%7B&type=cs https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+root%5Cs%7B&type=cs Leaving as-is for now. Dan Beam 2017/04/04 19:53:37 https://developer.mozilla.org/en-US/docs/Web/CSS/: Show quoted text On 2017/04/04 19:08:33, proberge wrote: > On 2017/04/04 17:40:57, Dan Beam (slow) wrote: > > fwiw: :root and html are probably equivalent here > > There's no uses of --my_css_rule: foo within html or body style tags, and a few > uses within root: > > https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+html%5Cs%7B&type=cs > https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+root%5Cs%7B&type=cs > > Leaving as-is for now. > https://developer.mozilla.org/en-US/docs/Web/CSS/:root proberge 2017/04/04 20:48:16 I'm sorry, my comment was a bit unclear. I wasn't Show quoted text On 2017/04/04 19:53:37, Dan Beam (slow) wrote: > On 2017/04/04 19:08:33, proberge wrote: > > On 2017/04/04 17:40:57, Dan Beam (slow) wrote: > > > fwiw: :root and html are probably equivalent here > > > > There's no uses of --my_css_rule: foo within html or body style tags, and a > few > > uses within root: > > > > > https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+html%5Cs%7B&type=cs > > > https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+root%5Cs%7B&type=cs > > > > Leaving as-is for now. > > > > https://developer.mozilla.org/en-US/docs/Web/CSS/:root I'm sorry, my comment was a bit unclear. I wasn't implying that you were wrong. The two are indeed equivalent. Since the two are equivalent, I'm leaning towards using the option which is consistent with other WebUIs. Please let me know if you would prefer to have the rules inside the html block. Dan Beam 2017/04/04 21:07:14 you obviously didn't want to do this: :root { - Show quoted text On 2017/04/04 20:48:16, proberge wrote: > On 2017/04/04 19:53:37, Dan Beam (slow) wrote: > > On 2017/04/04 19:08:33, proberge wrote: > > > On 2017/04/04 17:40:57, Dan Beam (slow) wrote: > > > > fwiw: :root and html are probably equivalent here > > > > > > There's no uses of --my_css_rule: foo within html or body style tags, and a > > few > > > uses within root: > > > > > > > > > https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+html%5Cs%7B&type=cs > > > > > > https://cs.chromium.org/search/?q=f:resources/+md_+%5C-%5C-+root%5Cs%7B&type=cs > > > > > > Leaving as-is for now. > > > > > > > https://developer.mozilla.org/en-US/docs/Web/CSS/:root > > I'm sorry, my comment was a bit unclear. I wasn't implying that you were wrong. > The two are indeed equivalent. > Since the two are equivalent, I'm leaning towards using the option which is > consistent with other WebUIs. Please let me know if you would prefer to have the > rules inside the html block. you obviously didn't want to do this: :root { --cleanup-card-margin: 25px; } :root { --cleanup-card-width: 800px; } and neither do "other WebUIs". this is basically what your code is equivalent to in my head right now: just extra, unnecessary selectors. if you see other stuff using both :root {} and html {} from a top-level context (like this), we should fix those, too
	20 --cleanup-card-margin: 25px;

	21 --cleanup-card-width: 800px;

	22 }

	23 </style>

	24 </head>

	25

	26 <body>

	27 <cleanup-tool-toolbar></cleanup-tool-toolbar>

	28 <cleanup-manager></cleanup-manager>

	29 <link rel="import" href="chrome://resources/html/cr.html">

	30 <link rel="import" href="chrome://resources/html/load_time_data.html">

	31 <link rel="import" href="chrome://resources/html/polymer.html">

	32 <link rel="import" href="chrome://cleanup/toolbar.html">

	33 <script src="strings.js"></script>

	34 </body>

	35

	36 </html>

OLD	NEW

« chrome/browser/browser_resources.grd ('K') | « chrome/browser/browser_resources.grd ('k') | chrome/browser/resources/cleanup_tool/compiled_resources2.gyp » ('j') | no next file with comments »