Merge lp://staging/~gary/juju-gui/placeholder into lp://staging/juju-gui/experimental

Please take a look.

Reviewers: mp+131788_code.launchpad.net,

Message:
Please take a look.

Description:
Change charm search to use HTML5 placeholder

CSS does not allow you to specify a selector for a dynamic empty (or not
empty) field value. The two approaches around it I found are JS and
required/:valid. I chose required/:valid because it required no JS and
seemed simpler. However, this required a CSS workaround: bootstrap
wants to highlight focused invalid fields with a red glow. I changed
this to have the usual blue glow in the case of this one field.

Thanks

Gary

https://code.launchpad.net/~gary/juju-gui/placeholder/+merge/131788

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6816046/

Affected files:
   A [revision details]
   M app/index.html
   M app/views/charm-search.js
   M lib/views/stylesheet.less

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: app/index.html
=== modified file 'app/index.html'
--- app/index.html 2012-10-13 23:56:14 +0000
+++ app/index.html 2012-10-28 16:34:34 +0000
@@ -53,7 +53,8 @@
                        Charms
                        <i class="icon-chevron-down"></i>
                      </span>
- <input type="text" id="charm-search-field"
value="Search for a charm" />
+ <input type="text" id="charm-search-field"
+ required="required" placeholder="Search for a charm"
/>
                    </span>
                  </span>
                </div>

Index: app/views/charm-search.js
=== modified file 'app/views/charm-search.js'
--- app/views/charm-search.js 2012-10-26 12:07:08 +0000
+++ app/views/charm-search.js 2012-10-28 16:34:34 +0000
@@ -550,23 +550,8 @@
        }
      };

- var handleFocus = function(ev) {
- if (ev.target.get('value').trim() === 'Search for a charm') {
- ev.target.set('value', '');
- }
- };
-
- var handleBlur = function(ev) {
- if (ev.target.get('value').trim() === '') {
- ev.target.set('value', 'Search for a charm');
- charmsSearchPanel.set('searchText', '');
- }
- };
-
      if (searchField) {
        searchField.on('keydown', handleKeyDown);
- searchField.on('blur', handleBlur);
- searchField.on('focus', handleFocus);
      }

      // The public methods

Index: lib/views/stylesheet.less
=== modified file 'lib/views/stylesheet.less'
--- lib/views/stylesheet.less 2012-10-26 06:33:29 +0000
+++ lib/views/stylesheet.less 2012-10-28 16:34:34 +0000
@@ -89,13 +89,19 @@
                      font-style: italic;
                      font-family: @font-family;
                      font-size: 0.9em;
- &:focus {
+ &:valid {
                          background: white;
                          padding: 3px;
                          width: 151px;
                          font-style: normal;
                    ...

Nice change Gary both in effect and implementation. Thanks. I hope it
can land very soon.

https://codereview.appspot.com/6816046/diff/1/app/index.html
File app/index.html (right):

https://codereview.appspot.com/6816046/diff/1/app/index.html#newcode57
app/index.html:57: required="required" placeholder="Search for a charm"
/>
Nice improvement! I was afraid we were going to have to wait for DOM
ready events to re-enable the field in order to solve this problem.
Glad you figured out the True solution.

https://codereview.appspot.com/6816046/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6816046/diff/1/lib/views/stylesheet.less#newcode103
lib/views/stylesheet.less:103: box-shadow: inset 0 1px 1px rgba(0, 0, 0,
0.075), 0 0 8px rgba(82, 168, 236, 0.6);
This line should be wrapped.

https://codereview.appspot.com/6816046/

Well played. I didn't remember that exists, but it does raise the
question of browser support again. IE 10 is supposed to handle
placeholder for example but previous version don't according to the
google, still, +1

https://codereview.appspot.com/6816046/

Thank you both, Ben and Brad.

https://codereview.appspot.com/6816046/diff/1/lib/views/stylesheet.less
File lib/views/stylesheet.less (right):

https://codereview.appspot.com/6816046/diff/1/lib/views/stylesheet.less#newcode103
lib/views/stylesheet.less:103: box-shadow: inset 0 1px 1px rgba(0, 0, 0,
0.075), 0 0 8px rgba(82, 168, 236, 0.6);
On 2012/11/01 09:47:56, bac wrote:
> This line should be wrapped.

Yes, thanks, did this one and line 86 above while I was at it.

https://codereview.appspot.com/6816046/

*** Submitted:

Change charm search to use HTML5 placeholder

CSS does not allow you to specify a selector for a dynamic empty (or not
empty) field value. The two approaches around it I found are JS and
required/:valid. I chose required/:valid because it required no JS and
seemed simpler. However, this required a CSS workaround: bootstrap
wants to highlight focused invalid fields with a red glow. I changed
this to have the usual blue glow in the case of this one field.

R=bac, benjamin.saller
CC=
https://codereview.appspot.com/6816046

https://codereview.appspot.com/6816046/