r97557 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r97556‎ | r97557 | r97558 >
Date:21:00, 19 September 2011
Author:preilly
Status:old (Comments)
Tags:
Comment:
mft r97556
Modified paths:
  • /branches/wmf/1.17wmf1/extensions/MobileFrontend/javascripts/application.js (modified) (history)
  • /branches/wmf/1.17wmf1/extensions/MobileFrontend/views/layout/application.html.php (modified) (history)

Diff [purge]

Index: branches/wmf/1.17wmf1/extensions/MobileFrontend/javascripts/application.js
@@ -1,49 +1,74 @@
2 -// Ideally, this would be loaded in the head and activated by a script at the bottom,
3 -// I think, rather than attached to an onload...
4 -window.onload = function() {
5 - // I don't think this makes sense. Loading this here means that this button
6 - // won't function until the page is loaded. It would probably be an
7 - // improvement to just attach an onclick straight into the html.
8 - document.getElementById( 'logo' ).onclick = function() {
9 - var n = document.getElementById( 'nav' ).style;
10 - n.display = n.display == 'block' ? 'none' : 'block';
11 - };
 2+var search = document.getElementById( 'search' );
 3+var clearSearch = document.getElementById( 'clearsearch' );
124
13 - // Also problematic, not working until the page loads...
14 - for( var h = document.getElementsByTagName( 'h2' ), i = 0; i < h.length; i++ ) {
15 - if ( h[i].className == 'section_heading' ) {
16 - h[i].onclick = function() {
17 - var section_idx = parseInt( this.id.replace( /section_(\d+)/, '$1' ) );
18 - wm_toggle_section( section_idx );
19 - }
20 - }
21 - };
 5+initClearSearchLink();
226
23 - // And this...
24 - for ( var a = document.getElementsByTagName( 'a' ), i = 0; i < a.length; i++ ) {
25 - a[i].onclick = function() {
26 - if ( this.hash.indexOf( '#' ) == 0 ) {
27 - wm_reveal_for_hash( this.hash );
28 - }
29 - }
30 - };
 7+function initClearSearchLink() {
 8+ clearSearch.setAttribute( 'title','Clear' );
 9+ clearSearch.addEventListener( 'mousedown', clearSearchBox, true );
 10+ search.addEventListener( 'keyup', _handleClearSearchLink, false );
 11+}
3112
32 - if ( document.location.hash.indexOf( '#' ) == 0 ) {
33 - wm_reveal_for_hash( document.location.hash );
 13+function _handleClearSearchLink() {
 14+ if ( clearSearch ) {
 15+ if ( search.value.length > 0 ) {
 16+ clearSearch.style.display = 'block';
 17+ } else {
 18+ clearSearch.style.display = 'none';
 19+ }
3420 }
 21+}
3522
36 - updateOrientation();
 23+function clearSearchBox( event ) {
 24+ search.value = '';
 25+ clearSearch.style.display = 'none';
 26+ if ( event ) {
 27+ event.preventDefault();
 28+ }
 29+}
3730
38 - // Try to scroll and hide URL bar
39 - window.scrollTo( 0, 1 );
 31+// I don't think this makes sense. Loading this here means that this button
 32+// won't function until the page is loaded. It would probably be an
 33+// improvement to just attach an onclick straight into the html.
 34+document.getElementById( 'logo' ).onclick = function() {
 35+ var n = document.getElementById( 'nav' ).style;
 36+ n.display = n.display == 'block' ? 'none' : 'block';
 37+};
4038
41 - // This is a global. I don't know why.
42 - decode = document.getElementById( 'searchField' );
43 - decode.value = unescape( decode.value );
44 - decode = document.getElementsByTagName( 'title' )[0];
45 - decode.innerHTML = unescape( decode.innerHTML );
 39+// Also problematic, not working until the page loads...
 40+for( var h = document.getElementsByTagName( 'h2' ), i = 0; i < h.length; i++ ) {
 41+ if ( h[i].className == 'section_heading' ) {
 42+ h[i].onclick = function() {
 43+ var section_idx = parseInt( this.id.replace( /section_(\d+)/, '$1' ) );
 44+ wm_toggle_section( section_idx );
 45+ }
 46+ }
4647 };
4748
 49+// And this...
 50+for ( var a = document.getElementsByTagName( 'a' ), i = 0; i < a.length; i++ ) {
 51+ a[i].onclick = function() {
 52+ if ( this.hash.indexOf( '#' ) == 0 ) {
 53+ wm_reveal_for_hash( this.hash );
 54+ }
 55+ }
 56+};
 57+
 58+if ( document.location.hash.indexOf( '#' ) == 0 ) {
 59+ wm_reveal_for_hash( document.location.hash );
 60+}
 61+
 62+updateOrientation();
 63+
 64+// Try to scroll and hide URL bar
 65+window.scrollTo( 0, 1 );
 66+
 67+// This is a global. I don't know why.
 68+decode = document.getElementById( 'search' );
 69+decode.value = unescape( decode.value );
 70+decode = document.getElementsByTagName( 'title' )[0];
 71+decode.innerHTML = unescape( decode.innerHTML );
 72+
4873 /**
4974 * updateOrientation checks the current orientation, sets the body's class
5075 * attribute to portrait, landscapeLeft, or landscapeRight,
Index: branches/wmf/1.17wmf1/extensions/MobileFrontend/views/layout/application.html.php
@@ -34,7 +34,6 @@
3535 }
3636 //]]>
3737 </script>
38 - <script type="text/javascript" language="javascript" src="{$wgExtensionAssetsPath}/MobileFrontend/javascripts/application.js?version=20110912T172820Z"></script>
3938 </head>
4039 <body>
4140 {$searchWebkitHtml}
@@ -43,6 +42,7 @@
4443 {$contentHtml}
4544 </div>
4645 {$footerHtml}
 46+ <script type="text/javascript" language="javascript" src="{$wgExtensionAssetsPath}/MobileFrontend/javascripts/application.js?version=20110919T172820Z"></script>
4747 </body>
4848 </html>
4949 EOT;

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r97556fix for Bug 30985 - Mobile header expanding and menu do not work until page l...preilly20:58, 19 September 2011

Comments

#Comment by Krinkle (talk | contribs)   21:05, 19 September 2011

I don't know the urgency or context behind this, but usually the trunk revision is reviewed before merging to production.

#Comment by 😂 (talk | contribs)   22:07, 19 September 2011

Yes, all revs should be reviewed before merging to the production branch. If it's a minor bugfix or enhancement, then there's no reason you shouldn't wait for a review. The only exception is merging a breakfix for something that's causing major issues (fatals, etc) on the live sites and needs to be fixed right now.

#Comment by Yair rand (talk | contribs)   05:44, 20 September 2011

event.preventDefault isn't supported by all browsers.

Status & tagging log