chrome/browser/resources/print_preview/search/search_box.html - Issue 10450022: Print Preview Print Destination Search Widget

Side by Side Diff

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

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: chrome/browser/resources/print_preview/search/search_box.html

Issue 10450022: Print Preview Print Destination Search Widget (Closed) Base URL: http://git.chromium.org/chromium/src.git@master

Patch Set: Address comments round 2 Created 8 years, 6 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:

View unified diff | Download patch

« chrome/browser/resources/print_preview/search/destination_search.html ('K') | « chrome/browser/resources/print_preview/search/search_box.css ('k') | chrome/browser/resources/print_preview/search/search_box.js » ('j') | chrome/browser/resources/print_preview/settings/destination_settings.css » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <div id="search-box" class="search-box">
	Dan Beam 2012/05/29 21:25:50 any reason to have both a class and ID? do you hav any reason to have both a class and ID? do you have multiple search-box's on the page or plan to have more? Robert Toscano 2012/05/29 22:17:36 The ID is there because I actually reference the e Show quoted text On 2012/05/29 21:25:50, Dan Beam wrote: > any reason to have both a class and ID? do you have multiple search-box's on the > page or plan to have more? The ID is there because I actually reference the element in JS by ID, and the class is there so that I can style it. I know that I can style based on the ID, but really the style is based on the class. Right now, this is a unique element, but I wanted to leave room for it being used in multiple places.
	2 <img src="../images/search.png" class="search-box-icon"/>

	3 <input type="text"
	Dan Beam 2012/05/29 21:25:50 seems like this wrapping is taking up too many lin seems like this wrapping is taking up too many lines, think the preferred is <input type="text" class="search-box-input" i18n-values="placeholder:searchBoxPlaceholder"/> also, you don't really need the / in self-closing tags in HTML (just XHTML), but that's another religious debate... Robert Toscano 2012/05/29 22:17:36 Done. Show quoted text On 2012/05/29 21:25:50, Dan Beam wrote: > seems like this wrapping is taking up too many lines, think the preferred is > > <input type="text" class="search-box-input" > i18n-values="placeholder:searchBoxPlaceholder"/> Done. Show quoted text > > also, you don't really need the / in self-closing tags in HTML (just XHTML), but > that's another religious debate... Yea I know I don't need "/" for HTML, but if we ever wanted to migrate to XHTML, it would be one less step. I also would just always end with "/" just so I don't ever leave it out when I shouldn't.
	4 class="search-box-input"

	5 i18n-values="placeholder:searchBoxPlaceholder"/>

	6 </div>

OLD	NEW