Code review comment for lp://staging/~gary/juju-gui/placeholder

Revision history for this message
Gary Poster (gary) wrote :

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;
                          font-size: 1em;
                      }
+ &:focus {
+ border-color: rgba(82, 168, 236, 0.8);
+ outline: 0;
+ outline: thin dotted 9;
+ box-shadow: inset 0 1px 1px rgba(0, 0, 0, 0.075),
0 0 8px rgba(82, 168, 236, 0.6);
+ }
                  }

              }

« Back to merge proposal