r109368 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r109367‎ | r109368 | r109369 >
Date:15:18, 18 January 2012
Author:seanheavey
Status:resolved (Comments)
Tags:jsplural 
Comment:
Development of feedback page style sheet.
Modified paths:
  • /trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.startup.js (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/images/bg-feedbackpage-header.png (added) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css (modified) (history)
  • /trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.css (modified) (history)

Diff [purge]

Index: trunk/extensions/ArticleFeedbackv5/ArticleFeedbackv5.i18n.php
——@@ -93,7 +93,7 @@
9494 'articlefeedbackv5-discussion-page' => 'Discussion',
9595 'articlefeedbackv5-whats-this' => 'Help',
9696 'articlefeedbackv5-invalid-page-id' => 'Invalid page ID',
97 - 'articlefeedbackv5-percent-found' => '<span class="stat-marker positive">$1%</span> of users found what they were looking for.',
 97+ 'articlefeedbackv5-percent-found' => '&nbsp;of users found what they were looking for.', // TODO: yeah, i know this is awful.
9898 'articlefeedbackv5-overall-rating' => 'Rating: $1/5',
9999 'articlefeedbackv5-special-title' => '==Feedback==',
100100 'articlefeedbackv5' => 'Article feedback dashboard',
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/images/bg-feedbackpage-header.png
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream
Property changes on: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/images/bg-feedbackpage-header.png
___________________________________________________________________
Added: svn:mime-type
101101 + application/octet-stream
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.css
——@@ -1,3 +1,183 @@
22 /* css here */
3 -div.articleFeedbackv5-feedback { border:1px solid #CCC; background:#EEE; margin-top:10px; clear:both;}
4 -div.articleFeedbackv5-feedback-tools { border:1px solid #CCD; background:#EEF; margin:10px; width:219px; float:right;}
 3+/*div.articleFeedbackv5-feedback { border:1px solid #CCC; background:#EEE; margin-top:10px; clear:both;}
 4+div.articleFeedbackv5-feedback-tools { border:1px solid #CCD; background:#EEF; margin:10px; width:219px; float:right;}*/
 5+/*----- FEEDBACK PAGE ------*/
 6+.float-clear:after {
 7+ clear: both;
 8+}
 9+#articleFeedbackv5-header-links {
 10+ float: right;
 11+}
 12+#articleFeedbackv5-percent-found-wrap {
 13+ float: left;
 14+}
 15+#articleFeedbackv5-showing-count-wrap {
 16+ clear: both;
 17+ float: right;
 18+ display: none;
 19+}
 20+#aftv5-feedbackpage .aftv5-feedbackpage-statistics {
 21+ float: left;
 22+}
 23+.aftv5-feedbackpage .aftv5-feedbackpage-statistics .stat-marker {
 24+ display: inline-block;
 25+ background: #2f8035;
 26+ color: #fff;
 27+ -webkit-border-radius: 5px;
 28+ -moz-border-radius: 5px;
 29+ border-radius: 5px;
 30+ width: 45px;
 31+ height: 30px;
 32+ line-height: 35px;
 33+ text-align: center;
 34+}
 35+.aftv5-feedbackpage .aftv5-feedbackpage-new-feedback {
 36+ float: right;
 37+}
 38+.aftv5-feedbackpage .aftv5-feedbackpage-comment-display-header {
 39+ height: 40px;
 40+ background: url(images/bg-feedbackpage-header.png) repeat-x;
 41+ padding: 0 10px;
 42+}
 43+#bodyContent h2 {
 44+ clear: both;
 45+}
 46+#articleFeedbackv5-sort-filter-controls {
 47+ padding: 0 10px;
 48+ height: 40px;
 49+ line-height: 40px;
 50+ background: url(images/bg-feedbackpage-header.png) repeat-x;
 51+}
 52+#articleFeedbackv5-sort-filter-controls:after {
 53+ clear: both;
 54+}
 55+#articleFeedbackv5-sort-filter-controls #articleFeedbackv5-sort {
 56+ float: left;
 57+ margin: 0 50px 0 0;
 58+}
 59+#articleFeedbackv5-sort-filter-controls #articleFeedbackv5-filter {
 60+ float: left;
 61+}v
 62+#articleFeedbackv5-sort-filter-controls select {
 63+ position: relative;
 64+ top: 10px;
 65+}
 66+#articleFeedbackv5-show-feedback {
 67+ overflow: scroll;
 68+ height: 600px;
 69+}
 70+.articleFeedbackv5-feedback {
 71+ position: relative;
 72+ padding: 10px 10px 09px;
 73+ border-bottom: 1px solid #d6d6d6;
 74+ overflow: hidden;
 75+}
 76+.articleFeedbackv5-feedback .articleFeedbackv5-feedback-tools {
 77+ display: none;
 78+ position: absolute;
 79+ top: 0;
 80+ right: 20px;
 81+ width: 140px;
 82+}
 83+.articleFeedbackv5-feedback .articleFeedbackv5-feedback-tools h3 {
 84+ background: #3366bb;
 85+ -moz-border-radius-bottomright: 5px;
 86+ -moz-border-radius-bottomleft: 5px;
 87+ -webkit-border-radius-bottomleft: 5px;
 88+ -webkit-border-radius-bottomright: 5px;
 89+ -ms-border-radius-bottomleft: 5px;
 90+ -ms-border-radius-bottomright: 5px;
 91+ border-radius-bottomleft: 5px;
 92+ border-radius-bottomright: 5px;
 93+ color: #fff;
 94+ text-align: center;
 95+ position: absolute;
 96+ z-index: 150;
 97+ width: 140px;
 98+ margin: 0;
 99+ cursor: pointer;
 100+}
 101+.articleFeedbackv5-feedback .articleFeedbackv5-feedback-tools ul {
 102+ position: relative;
 103+ padding: 40px 10px 10px;
 104+ background: #d2e5f7;
 105+ -moz-border-radius-bottomright: 5px;
 106+ -moz-border-radius-bottomleft: 5px;
 107+ -webkit-border-radius-bottomleft: 5px;
 108+ -webkit-border-radius-bottomright: 5px;
 109+ -ms-border-radius-bottomleft: 5px;
 110+ -ms-border-radius-bottomright: 5px;
 111+ border-radius-bottomleft: 5px;
 112+ border-radius-bottomright: 5px;
 113+ margin: 0;
 114+ list-style: none;
 115+}
 116+.articleFeedbackv5-feedback .articleFeedbackv5-feedback-tools ul li a:hover {
 117+ cursor: pointer;
 118+}
 119+.articleFeedbackv5-feedback:hover {
 120+ padding: 9px 10px;
 121+ border-top: 1px solid #3366bb;
 122+ border-bottom: 1px solid #3366bb;
 123+}
 124+.articleFeedbackv5-feedback:hover .articleFeedbackv5-feedback-tools {
 125+ display: block;
 126+}
 127+span.positive {
 128+ color: #2f8035;
 129+}
 130+a#articleFeedbackv5-show-more {
 131+ display: block;
 132+ width: 100%;
 133+ height: 40px;
 134+ line-height: 40px;
 135+ text-align: center;
 136+ background: #d2e5f7;
 137+}
 138+.articleFeedbackv5-feedback blockquote {
 139+ float: left;
 140+}
 141+.articleFeedbackv5-feedback .articleFeedbackv5-comment-details {
 142+ float: right;
 143+ clear: both;
 144+}
 145+#articleFeedbackv5-special-add-feedback {
 146+ -webkit-box-shadow:#999999 -2px 2px 1px;
 147+ background-attachment:initial !important;
 148+ background-clip:initial !important;
 149+ background-color:initial !important;
 150+ background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAAjCAIAAADaE/fjAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAyRpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdpbj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuMC1jMDYxIDY0LjE0MDk0OSwgMjAxMC8xMi8wNy0xMDo1NzowMSAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtcDpDcmVhdG9yVG9vbD0iQWRvYmUgUGhvdG9zaG9wIENTNS4xIE1hY2ludG9zaCIgeG1wTU06SW5zdGFuY2VJRD0ieG1wLmlpZDo4QzkwMjE1MjE0RTExMUUxQTkyM0IxNzE3N0RFODgwNiIgeG1wTU06RG9jdW1lbnRJRD0ieG1wLmRpZDo4MTMwRTQxMjE0RTIxMUUxQTkyM0IxNzE3N0RFODgwNiI+IDx4bXBNTTpEZXJpdmVkRnJvbSBzdFJlZjppbnN0YW5jZUlEPSJ4bXAuaWlkOjhDOTAyMTUwMTRFMTExRTFBOTIzQjE3MTc3REU4ODA2IiBzdFJlZjpkb2N1bWVudElEPSJ4bXAuZGlkOjhDOTAyMTUxMTRFMTExRTFBOTIzQjE3MTc3REU4ODA2Ii8+IDwvcmRmOkRlc2NyaXB0aW9uPiA8L3JkZjpSREY+IDwveDp4bXBtZXRhPiA8P3hwYWNrZXQgZW5kPSJyIj8+OdQuLwAAAEFJREFUeNpcjcENACAIAysrOZNLuputoWo0vi5HKaC2HpBCUBSTdoieOROXPyzmzaCxfTy7XzfvMbm6SVDnzxRgACWnTTX5Rk3FAAAAAElFTkSuQmCC) !important;
 151+ background-origin:initial !important;
 152+ background-position:initial initial !important;
 153+ background-repeat:repeat no-repeat !important;
 154+ border-bottom-left-radius:3px;
 155+ border-bottom-right-radius:3px;
 156+ border-bottom-style:none !important;
 157+ border-color:initial !important;
 158+ border-image:initial !important;
 159+ border-left-style:none !important;
 160+ border-right-style:none !important;
 161+ border-top-left-radius:3px;
 162+ border-top-right-radius:3px;
 163+ border-top-style:none !important;
 164+ border-width:initial !important;
 165+ box-shadow:#999999 -2px 2px 1px;
 166+ background-attachment:scroll !important;
 167+ background-clip:initial !important;
 168+ background-color:#407EC9 !important;
 169+ background-origin:initial !important;
 170+ background-position:50% 100% !important;
 171+ background-repeat:repeat no-repeat !important;
 172+ border-bottom-color:#407EC9 !important;
 173+ border-left-color:#407EC9 !important;
 174+ border-right-color:#407EC9 !important;
 175+ border-top-color:#407EC9 !important;
 176+ color:white;
 177+ display: block;
 178+ width: auto;
 179+ height: 35px;
 180+ line-height: 35px;
 181+ padding: 0 9px;
 182+
 183+ float: right;
 184+}
\ No newline at end of file
Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.css
——@@ -203,24 +203,24 @@
204204 /* @embed */
205205 background-image: url(images/trash-hover.png);
206206 }
207 -.articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-full {
 207+.articleFeedbackv5-rating-label-full {
208208 /* @embed */
209209 background-image: url(images/star-full.png);
210210 }
211 -.articleFeedbackv5-expired .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-full {
 211+.articleFeedbackv5-expired .articleFeedbackv5-rating-label-full {
212212 /* @embed */
213213 background-image: url(images/star-full-expired.png);
214214 }
215 -.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-full,
216 -.articleFeedbackv5-rating .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-hover-tail {
 215+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-full,
 216+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-hover-tail {
217217 /* @embed */
218218 background-image: url(images/star-new.png);
219219 }
220 -.articleFeedbackv5-rating .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-hover-head {
 220+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-hover-head {
221221 /* @embed */
222222 background-image: url(images/star-new-hover.png);
223223 }
224 -.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-down {
 224+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-down {
225225 /* @embed */
226226 background-image: url(images/star-new-down.png);
227227 }
——@@ -230,24 +230,24 @@
231231 /* @embed */
232232 background-image: url(images/old-star-empty.png);
233233 }
234 -.articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-full {
 234+.articleFeedbackv5-rating-label-full {
235235 /* @embed */
236236 background-image: url(images/old-star-full.png);
237237 }
238 -.articleFeedbackv5-expired .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-full {
 238+.articleFeedbackv5-expired .articleFeedbackv5-rating-label-full {
239239 /* @embed */
240240 background-image: url(images/old-star-full-expired.png);
241241 }
242 -.articleFeedbackv5-option-5 .articleFeedbackv5-rating-new .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-full,
243 -.articleFeedbackv5-option-5 .articleFeedbackv5-rating .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-hover-tail {
 242+.articleFeedbackv5-option-5 .articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-full,
 243+.articleFeedbackv5-option-5 .articleFeedbackv5-rating .articleFeedbackv5-rating-label-hover-tail {
244244 /* @embed */
245245 background-image: url(images/old-star-new.png);
246246 }
247 -.articleFeedbackv5-option-5 .articleFeedbackv5-rating .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-hover-head {
 247+.articleFeedbackv5-option-5 .articleFeedbackv5-rating .articleFeedbackv5-rating-label-hover-head {
248248 /* @embed */
249249 background-image: url(images/old-star-new-hover.png);
250250 }
251 -.articleFeedbackv5-option-5 .articleFeedbackv5-rating-new .articleFeedbackv5-rating-label.articleFeedbackv5-rating-label-down {
 251+.articleFeedbackv5-option-5 .articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-down {
252252 /* @embed */
253253 background-image: url(images/old-star-new-down.png);
254254 }
——@@ -789,16 +789,21 @@
790790 /*** Option 3 ***/
791791 .articleFeedbackv5-rating-new .articleFeedbackv5-rating-label {
792792 /* @embed */
793 - background: url(images/bg-rating-off.png) no-repeat;
 793+ background: url(images/star-empty.png) no-repeat;
794794 }
795 -
796 -.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label.active {
 795+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-full {
797796 /* @embed */
798 - background: url(images/bg-rating-on.png) no-repeat;
 797+ background: url(images/star-new.png) no-repeat;
799798 }
800 -
801 -.articleFeedbackv5-rating .articleFeedbackv5-rating-prompt {
 799+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-hover-head,
 800+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-hover-tail {
 801+ /* @embed */
 802+ background: url(images/star-new-hover.png) no-repeat;
802803 }
 804+.articleFeedbackv5-rating-new .articleFeedbackv5-rating-label-hover-head:active {
 805+ /* @embed */
 806+ background: url(images/star-new-down.png) no-repeat;
 807+}
803808
804809 /*** Option 4 ***/
805810 .articleFeedbackv5-option-4 .articleFeedbackv5-submit {
Index: trunk/extensions/ArticleFeedbackv5/modules/ext.articleFeedbackv5/ext.articleFeedbackv5.startup.js
——@@ -20,7 +20,7 @@
2121 // Rule out MSIE 6, iPhone, iPod, iPad, Android
2222 if(
2323 (ua.indexOf( 'msie 6' ) != -1) ||
24 - (ua.indexOf( 'msie 7' ) != -1) ||
 24+ /*(ua.indexOf( 'msie 7' ) != -1) ||*/
2525 (ua.indexOf( 'firefox/2') != -1) ||
2626 (ua.indexOf( 'firefox 2') != -1) ||
2727 (ua.indexOf( 'android' ) != -1) ||

Follow-up revisions

RevisionCommit summaryAuthorDate
r109390Fix AFT5 special page translation again - looks like it got clobbered in r109...gregchiasson16:46, 18 January 2012
r110349Fixed a missing embed; not sure what revision, possibly r109368rsterbin00:32, 31 January 2012

Comments

#Comment by Reedy (talk | contribs)   16:25, 18 January 2012 

Index: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/images/bg-feedbackpage-header.png
===================================================================
Cannot display: file marked as a binary type.
svn:mime-type = application/octet-stream

Property changes on: trunk/extensions/ArticleFeedbackv5/modules/jquery.articleFeedbackv5/images/bg-feedbackpage-header.png
___________________________________________________________________
Added: svn:mime-type
   + application/octet-stream

Need to fix your Subversion/auto-props

#Comment by Johnduhart (talk | contribs)   19:12, 23 January 2012

Missing /* @embed */s

#Comment by Catrope (talk | contribs)   01:48, 26 January 2012 
+  background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAAjCAIAAADaE/fjAAAAGXRFWHRTb2Z0d2FyZQBBZG9iZSBJbWFnZVJlYWR5ccllPAAAAyRpVFh0WE1MOmNvbS5hZG9iZS54bXAAAAAAADw/eHBhY2tldCBiZWdpbj0i77u/IiBpZD0iVzVNME1wQ2VoaUh6cmVTek5UY3prYzlkIj8+IDx4OnhtcG1ldGEgeG1sbnM6eD0iYWRvYmU6bnM6bWV0YS8iIHg6eG1wdGs9IkFkb2JlIFhNUCBDb3JlIDUuMC1jMDYxIDY0LjE0MDk0OSwgMjAxMC8xMi8wNy0xMDo1NzowMSAgICAgICAgIj4gPHJkZjpSREYgeG1sbnM6cmRmPSJodHRwOi8vd3d3LnczLm9yZy8xOTk5LzAyLzIyLXJkZi1zeW50YXgtbnMjIj4gPHJkZjpEZXNjcmlwdGlvbiByZGY6YWJvdXQ9IiIgeG1sbnM6eG1wPSJodHRwOi8vbnMuYWRvYmUuY29tL3hhcC8xLjAvIiB4bWxuczp4bXBNTT0iaHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wL21tLyIgeG1sbnM6c3RSZWY9Imh0dHA6Ly9ucy5hZG9iZS5jb20veGFwLzEuMC9zVHlwZS9SZXNvdXJjZVJlZiMiIHhtcDpDcmVhdG9yVG9vbD0iQWRvYmUgUGhvdG9zaG9wIENTNS4xIE1hY2ludG9zaCIgeG1wTU06SW5zdGFuY2VJRD0ieG1wLmlpZDo4QzkwMjE1MjE0RTExMUUxQTkyM0IxNzE3N0RFODgwNiIgeG1wTU06RG9jdW1lbnRJRD0ieG1wLmRpZDo4MTMwRTQxMjE0RTIxMUUxQTkyM0IxNzE3N0RFODgwNiI+IDx4bXBNTTpEZXJpdmVkRnJvbSBzdFJlZjppbnN0YW5jZUlEPSJ4bXAuaWlkOjhDOTAyMTUwMTRFMTExRTFBOTIzQjE3MTc3REU4ODA2IiBzdFJlZjpkb2N1bWVudElEPSJ4bXAuZGlkOjhDOTAyMTUxMTRFMTExRTFBOTIzQjE3MTc3REU4ODA2Ii8+IDwvcmRmOkRlc2NyaXB0aW9uPiA8L3JkZjpSREY+IDwveDp4bXBtZXRhPiA8P3hwYWNrZXQgZW5kPSJyIj8+OdQuLwAAAEFJREFUeNpcjcENACAIAysrOZNLuputoWo0vi5HKaC2HpBCUBSTdoieOROXPyzmzaCxfTy7XzfvMbm6SVDnzxRgACWnTTX5Rk3FAAAAAElFTkSuQmCC) !important;

WTF? Don't embed data URLs like that, save the image as an image file and use /* @embed */ , that way ResourceLoader will automatically do the data URL processing for you. Per John's comment, you should also add /* @embed */ for the other image file references you're adding. 

+  box-shadow:#999999 -2px 2px 1px;
+	background-attachment:scroll !important;
+  background-clip:initial !important;

Indentation is messed up.

The i18n change was reverted later, so that's fine.

Marking fixme for the data URL and @embed issues, the rest is OK.

#Comment by Rsterbin (talk | contribs)   00:34, 31 January 2012

These issues (i18n, indentation, embed, weird spacing) got fixed, although I'm not sure when. I don't think Sean doesn't have the right privs to set this back to new, so doing that for him now.

Status & tagging log