juju-gui

Merge lp:~huwshimi/juju-gui/yui3-skin-sam-removal into lp:juju-gui/experimental

  1. yui3-skin-sam-removal
  2. Merge into trunk
Proposed by Huw Wilkins
Status: Merged
Merged at revision: 1169
Proposed branch: lp:~huwshimi/juju-gui/yui3-skin-sam-removal
Merge into: lp:juju-gui/experimental
Diff against target: 283 lines (+102/-88)
7 files modified
app/index.html (+1/-1)
app/templates/right-sidebar.partial (+1/-1)
app/templates/service-inspector.handlebars (+1/-1)
bin/merge-files (+16/-1)
lib/views/browser/bws-searchbox.less (+3/-2)
lib/views/browser/tabview.less (+73/-75)
lib/views/stylesheet.less (+7/-7)
To merge this branch: bzr merge lp:~huwshimi/juju-gui/yui3-skin-sam-removal
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+193372@code.launchpad.net

Description of the change

Remove yui3-skin-sam

We were not really making use of the yui3-skin-sam classes, except in a few minor cases.

I've removed those classes from our HTML and fixed any fallout.

I've also updated our CSS merging script to manually include just the core CSS files that we require.

Rick suggested overriding the Loader to not pull in the sam skin files, but in the end I need to abandon the Loader's auto-discovered files as it still included files we did not want.

I've left a few comments in the merge file about including files, but if there are additional places I need to add documentation I'd be happy to do so.

https://codereview.appspot.com/20120044/

To post a comment you must log in.
Revision history for this message
Huw Wilkins (huwshimi) wrote :

Reviewers: mp+193372_code.launchpad.net,

Message:
Please take a look.

Description:
Remove yui3-skin-sam

We were not really making use of the yui3-skin-sam classes, except in a
few minor cases.

I've removed those classes from our HTML and fixed any fallout.

I've also updated our CSS merging script to manually include just the
core CSS files that we require.

Rick suggested overriding the Loader to not pull in the sam skin files,
but in the end I need to abandon the Loader's auto-discovered files as
it still included files we did not want.

I've left a few comments in the merge file about including files, but if
there are additional places I need to add documentation I'd be happy to
do so.

https://code.launchpad.net/~huwshimi/juju-gui/yui3-skin-sam-removal/+merge/193372

(do not edit description out of merge proposal)

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

Affected files (+104, -88 lines):
   A [revision details]
   M app/index.html
   M app/templates/right-sidebar.partial
   M app/templates/service-inspector.handlebars
   M bin/merge-files
   M lib/views/browser/bws-searchbox.less
   M lib/views/browser/tabview.less
   M lib/views/stylesheet.less

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

LGTM with a desire for a reply to my question sometime in the future.
:-)

Doing QA next.

https://codereview.appspot.com/20120044/diff/1/bin/merge-files
File bin/merge-files (right):

https://codereview.appspot.com/20120044/diff/1/bin/merge-files#newcode119
bin/merge-files:119: // don't get any yui3-skin-sam classes and ignore
file we don't need.
Do we have a better plan for the future here?

https://codereview.appspot.com/20120044/

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

QAOK. Looks good with uncompressed and compressed sources. Thank you!

Landing now.

Gary

https://codereview.appspot.com/20120044/

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

*** Submitted:

Remove yui3-skin-sam

We were not really making use of the yui3-skin-sam classes, except in a
few minor cases.

I've removed those classes from our HTML and fixed any fallout.

I've also updated our CSS merging script to manually include just the
core CSS files that we require.

Rick suggested overriding the Loader to not pull in the sam skin files,
but in the end I need to abandon the Loader's auto-discovered files as
it still included files we did not want.

I've left a few comments in the merge file about including files, but if
there are additional places I need to add documentation I'd be happy to
do so.

R=gary.poster
CC=
https://codereview.appspot.com/20120044

https://codereview.appspot.com/20120044/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'app/index.html'
2--- app/index.html 2013-10-30 12:54:32 +0000
3+++ app/index.html 2013-10-31 04:50:55 +0000
4@@ -168,7 +168,7 @@
5 <div id="content">
6 <div id="shortcut-help" style="display:none"></div>
7 <div id="subapp-browser-min" style="display: none;"></div>
8- <div id="subapp-browser" class="yui3-skin-sam"></div>
9+ <div id="subapp-browser"></div>
10 <div id="main">
11 </div> <!-- /container -->
12 </div>
13
14=== modified file 'app/templates/right-sidebar.partial'
15--- app/templates/right-sidebar.partial 2013-08-06 16:00:16 +0000
16+++ app/templates/right-sidebar.partial 2013-10-31 04:50:55 +0000
17@@ -1,4 +1,4 @@
18-<div class="controls yui3-skin-sam zoom-controls">
19+<div class="controls zoom-controls">
20 <div id="zoom-in-btn" class="sprite zoom_plus"></div>
21 <span id="slider-parent"></span>
22 <div id="zoom-out-btn" class="sprite zoom_minus"></div>
23
24=== modified file 'app/templates/service-inspector.handlebars'
25--- app/templates/service-inspector.handlebars 2013-09-23 00:44:11 +0000
26+++ app/templates/service-inspector.handlebars 2013-10-31 04:50:55 +0000
27@@ -1,4 +1,4 @@
28 <div class="yui3-juju-inspector">
29 <div class="panel juju-inspector"></div>
30- <div class="left-breakout content-panel yui3-skin-sam" style="display: none;"></div>
31+ <div class="left-breakout content-panel" style="display: none;"></div>
32 </div>
33
34=== modified file 'bin/merge-files'
35--- bin/merge-files 2013-10-24 21:33:48 +0000
36+++ bin/merge-files 2013-10-31 04:50:55 +0000
37@@ -108,10 +108,25 @@
38
39 merge.combineJs(filesToLoad.js, 'build-shared/juju-ui/assets/all-yui.js');
40
41- var cssFiles = filesToLoad.css;
42+ // We're not including the pre-discovered CSS files so we can only include
43+ // the CSS files we want.
44+ var cssFiles = [];
45 cssFiles.push('app/assets/stylesheets/normalize.css');
46 cssFiles.push('app/assets/stylesheets/prettify.css');
47 cssFiles.push('app/assets/stylesheets/cssgrids-responsive-min.css');
48+
49+ // Manually include YUI CSS files so we can just include the core files,
50+ // don't get any yui3-skin-sam classes and ignore file we don't need.
51+ cssFiles.push(
52+ 'node_modules/yui/app-transitions-css/app-transitions-css-min.css');
53+ cssFiles.push(
54+ 'node_modules/yui/autocomplete-list/assets/autocomplete-list-core.css');
55+ cssFiles.push('node_modules/yui/panel/assets/panel-core.css');
56+ cssFiles.push('node_modules/yui/slider-base/assets/slider-base-core.css');
57+ cssFiles.push('node_modules/yui/tabview/assets/tabview-core.css');
58+ cssFiles.push('node_modules/yui/widget-base/assets/widget-base-core.css');
59+ cssFiles.push('node_modules/yui/widget-stack/assets/widget-stack-core.css');
60+
61 merge.combineCSS(cssFiles,
62 'build-shared/juju-ui/assets/combined-css/all-static.css');
63 });
64
65=== modified file 'lib/views/browser/bws-searchbox.less'
66--- lib/views/browser/bws-searchbox.less 2013-10-30 12:16:58 +0000
67+++ lib/views/browser/bws-searchbox.less 2013-10-31 04:50:55 +0000
68@@ -33,6 +33,7 @@
69 .yui3-aclist {
70 z-index: 15;
71 width: 100%;
72+ background-color: #fff;
73
74 .yui3-aclist-content {
75 border: 1px solid #bbb;
76@@ -106,8 +107,8 @@
77 }
78 }
79
80-.yui3-skin-sam .yui3-aclist-item-hover {
81- background: rgb(238,238,238);
82+.yui3-aclist-item-hover {
83+ background-color: #eee;
84 }
85
86 #subapp-browser .browser-nav {
87
88=== modified file 'lib/views/browser/tabview.less'
89--- lib/views/browser/tabview.less 2013-10-29 02:29:58 +0000
90+++ lib/views/browser/tabview.less 2013-10-31 04:50:55 +0000
91@@ -1,79 +1,77 @@
92-.yui3-skin-sam {
93- .yui3-juju-browser-tabview {
94- .vertical {
95- ul {
96- float: left;
97- }
98- .yui3-juju-browser-tab {
99+.yui3-juju-browser-tabview {
100+ .vertical {
101+ ul {
102+ float: left;
103+ }
104+ .yui3-juju-browser-tab {
105+ display: block;
106+ }
107+ }
108+ .yui3-tabview-list {
109+ padding: 0 20px 0 @tabview-left-padding !important;
110+ border-bottom: 1px solid @bws-border;
111+
112+ li {
113+ a {
114+ border: none;
115+ }
116+ margin-right: 38px;
117+
118+ &:last-child {
119+ margin-right: 0;
120+ }
121+ &.yui3-tab-selected a.yui3-tab-label {
122+ background: inherit;
123+ border-bottom: 2px solid @charm-panel-orange;
124+ padding-bottom: 15px;
125+ }
126+ .yui3-tab-label {
127+ .type9;
128+ background: inherit;
129 display: block;
130- }
131- }
132- .yui3-tabview-list {
133- padding: 0 20px 0 @tabview-left-padding !important;
134- border-bottom: 1px solid @bws-border;
135-
136- li {
137- a {
138- border: none;
139- }
140- margin-right: 38px;
141-
142- &:last-child {
143- margin-right: 0;
144- }
145- &.yui3-tab-selected a.yui3-tab-label {
146- background: inherit;
147- border-bottom: 2px solid @charm-panel-orange;
148- padding-bottom: 15px;
149- }
150- .yui3-tab-label {
151- .type9;
152- background: inherit;
153- display: block;
154- padding: 16px 0;
155- outline: 0;
156- line-height: 18px;
157- }
158- }
159- }
160- .yui3-tabview-panel {
161- padding: 0;
162- background: inherit;
163- border: none;
164-
165- .yui3-tab-panel {
166- .type9;
167- .border-box;
168- padding: 0 70px 0 @tabview-left-padding;
169-
170- a:hover {
171- text-decoration: underline;
172- }
173- h1,
174- h2,
175- h3,
176- h4 {
177- .type5;
178- margin: 40px 0 20px;
179- padding-bottom: 0;
180- border-bottom: 0;
181- }
182- p {
183- line-height: 1.6;
184- }
185- ul, ol {
186- margin: 0;
187- padding: 0;
188- list-style: none;
189- }
190- pre {
191- background: #FDF6F2;
192- border-radius: @border-radius;
193- padding: 10px 16px;
194- white-space: pre-wrap;
195- word-wrap: break-word;
196- margin-bottom: 9px;
197- }
198+ padding: 16px 0;
199+ outline: 0;
200+ line-height: 18px;
201+ }
202+ }
203+ }
204+ .yui3-tabview-panel {
205+ padding: 0;
206+ background: inherit;
207+ border: none;
208+
209+ .yui3-tab-panel {
210+ .type9;
211+ .border-box;
212+ padding: 0 70px 0 @tabview-left-padding;
213+
214+ a:hover {
215+ text-decoration: underline;
216+ }
217+ h1,
218+ h2,
219+ h3,
220+ h4 {
221+ .type5;
222+ margin: 40px 0 20px;
223+ padding-bottom: 0;
224+ border-bottom: 0;
225+ }
226+ p {
227+ line-height: 1.6;
228+ }
229+ ul, ol {
230+ margin: 0;
231+ padding: 0;
232+ list-style: none;
233+ }
234+ pre {
235+ background: #FDF6F2;
236+ border-radius: @border-radius;
237+ padding: 10px 16px;
238+ white-space: pre-wrap;
239+ word-wrap: break-word;
240+ margin-bottom: 9px;
241 }
242 }
243 }
244
245=== modified file 'lib/views/stylesheet.less'
246--- lib/views/stylesheet.less 2013-10-30 12:54:32 +0000
247+++ lib/views/stylesheet.less 2013-10-31 04:50:55 +0000
248@@ -764,28 +764,28 @@
249 right: 20px;
250 top: 40px;
251 }
252-.yui3-skin-sam .yui3-slider-y .yui3-slider-rail,
253-.yui3-skin-sam .yui3-slider-y .yui3-slider-rail-cap-top,
254-.yui3-skin-sam .yui3-slider-y .yui3-slider-rail-cap-bottom {
255+.yui3-slider-y .yui3-slider-rail,
256+.yui3-slider-y .yui3-slider-rail-cap-top,
257+.yui3-slider-y .yui3-slider-rail-cap-bottom {
258 background-image: none;
259 }
260-.yui3-skin-sam .yui3-slider-y .yui3-slider-rail {
261+.yui3-slider-y .yui3-slider-rail {
262 .create-border-radius(@border-radius);
263 .create-box-shadow(inset 0 0 4px rgba(0,0,0,.51));
264 background-color: #bbbbbb;
265 width: 10px;
266 }
267
268-.yui3-skin-sam .yui3-slider-y .yui3-slider-thumb-shadow {
269+.yui3-slider-y .yui3-slider-thumb-shadow {
270 display: none;
271 }
272-.yui3-skin-sam .yui3-slider-y .yui3-slider-thumb {
273+.yui3-slider-y .yui3-slider-thumb {
274 width: 21px;
275 height: 21px;
276 left: 0;
277 overflow: visible;
278 }
279-.yui3-skin-sam .yui3-slider-y .yui3-slider-thumb-image {
280+.yui3-slider-y .yui3-slider-thumb-image {
281 left: -5px;
282 }
283 #zoom-in-btn,

Subscribers

People subscribed via source and target branches