Merge lp:~jblount/libubuntuone/classy-error-pages into lp:libubuntuone

Proposed by Joshua Blount
Status: Rejected
Rejected by: Stuart Langridge
Proposed branch: lp:~jblount/libubuntuone/classy-error-pages
Merge into: lp:libubuntuone
Diff against target: 621 lines (+521/-7)
7 files modified
data/Makefile.am (+4/-1)
data/connecting.html (+34/-0)
data/in_development.html (+34/-0)
data/load_error.html (+41/-0)
data/reset.css (+49/-0)
data/screen.css (+336/-0)
libubuntuone/u1-music-store.c (+23/-6)
To merge this branch: bzr merge lp:~jblount/libubuntuone/classy-error-pages
Reviewer Review Type Date Requested Status
Stuart Langridge (community) Disapprove
Rodrigo Moya (community) Needs Fixing
Eric Casteleijn (community) Approve
Review via email: mp+21513@code.launchpad.net

Description of the change

Adds some css and html to make the error pages look nice / right / good.

To post a comment you must log in.
Revision history for this message
Rodrigo Moya (rodrigo-moya) wrote :

Looks ok to me, although I just merged another branch that adds a new page (initial page when loading the real store), so I think this is going to have conflicts, so can you please merge with trunk and add that other page please?

Revision history for this message
Eric Casteleijn (thisfred) wrote :

Looks good, code builds.

review: Approve
55. By Joshua Blount

merged with trunk

Revision history for this message
Joshua Blount (jblount) wrote :

> Looks ok to me, although I just merged another branch that adds a new page
> (initial page when loading the real store), so I think this is going to have
> conflicts, so can you please merge with trunk and add that other page please?

I merged with trunk, but could you please once over it and make sure I didn't get rid of anything?

Revision history for this message
Rodrigo Moya (rodrigo-moya) wrote :

Ok, there are a few problems:

* in_development.html seems to be missing, right? Also, the call to load that file seems to be missing also.
* There are some leaks in the C code:

552 + calculated_file_url = g_strdup_printf("%s?%s",
553 + g_filename_to_uri(calculated_file_path, NULL, NULL),
554 + reload_url);

g_filename_to_uri returns a newly-allocated string, so you need to store it in a variable, use it to g_strdup_printf calculated_file_url, and then g_free it.

* also, we use the GNOME coding style for couchdb-glib (because it's going to be included in GNOME soon), so could you please add a space after the function name and before the '(' character?

Apart from that it looks ok, so please fix that and I'll approve it

review: Needs Fixing
56. By Joshua Blount

added in_development.html template

Revision history for this message
Stuart Langridge (sil) wrote :

Rejecting because instead we'll use https://code.edge.launchpad.net/~sil/libubuntuone/classy-error-pages/ which fixes Rodrigo's issues.

review: Disapprove

Unmerged revisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'data/Makefile.am'
2--- data/Makefile.am 2010-02-17 12:57:19 +0000
3+++ data/Makefile.am 2010-03-19 11:51:34 +0000
4@@ -2,6 +2,9 @@
5 js_DATA = \
6 u1-library-override.js \
7 u1-preview.js \
8- u1-songs-clickable.js
9+ u1-songs-clickable.js \
10+ load_error.html \
11+ connecting.html \
12+ staticstyle.css
13
14 EXTRA_DIST = $(js_DATA)
15
16=== added file 'data/connecting.html'
17--- data/connecting.html 1970-01-01 00:00:00 +0000
18+++ data/connecting.html 2010-03-19 11:51:34 +0000
19@@ -0,0 +1,34 @@
20+<!doctype html>
21+<html>
22+<head>
23+<title>Loading error</title>
24+<link rel="stylesheet" href="reset.css" type="text/css" media="all">
25+<link rel="stylesheet" href="screen.css" type="text/css" media="all">
26+
27+</head>
28+<body class="downloads">
29+<div id="header">
30+
31+<div class="wrap">
32+<h1 id="logo">Ubuntu One Music Store</h1>
33+
34+</div><!-- close .wrap -->
35+</div><!-- close #header -->
36+
37+<div class="superfluous">
38+<div id="content">
39+<div class="access">
40+ <h1>Connecting</h1>
41+<h2>o hai we are helping you connect to ubuntu one in an unstyled fashion</h2>
42+</div>
43+</div>
44+
45+</div>
46+</div>
47+<div id="footer">
48+<div class="wrap">
49+</div>
50+</div>
51+</body>
52+</html>
53+
54
55=== added directory 'data/img'
56=== added file 'data/img/button-left.png'
57Binary files data/img/button-left.png 1970-01-01 00:00:00 +0000 and data/img/button-left.png 2010-03-19 11:51:34 +0000 differ
58=== added file 'data/img/button-right.png'
59Binary files data/img/button-right.png 1970-01-01 00:00:00 +0000 and data/img/button-right.png 2010-03-19 11:51:34 +0000 differ
60=== added file 'data/img/content_top.png'
61Binary files data/img/content_top.png 1970-01-01 00:00:00 +0000 and data/img/content_top.png 2010-03-19 11:51:34 +0000 differ
62=== added file 'data/img/downloads_background.png'
63Binary files data/img/downloads_background.png 1970-01-01 00:00:00 +0000 and data/img/downloads_background.png 2010-03-19 11:51:34 +0000 differ
64=== added file 'data/img/footer_clouds.png'
65Binary files data/img/footer_clouds.png 1970-01-01 00:00:00 +0000 and data/img/footer_clouds.png 2010-03-19 11:51:34 +0000 differ
66=== added file 'data/img/green_check.png'
67Binary files data/img/green_check.png 1970-01-01 00:00:00 +0000 and data/img/green_check.png 2010-03-19 11:51:34 +0000 differ
68=== added file 'data/img/header_background.png'
69Binary files data/img/header_background.png 1970-01-01 00:00:00 +0000 and data/img/header_background.png 2010-03-19 11:51:34 +0000 differ
70=== added file 'data/img/logo.png'
71Binary files data/img/logo.png 1970-01-01 00:00:00 +0000 and data/img/logo.png 2010-03-19 11:51:34 +0000 differ
72=== added file 'data/img/menu.png'
73Binary files data/img/menu.png 1970-01-01 00:00:00 +0000 and data/img/menu.png 2010-03-19 11:51:34 +0000 differ
74=== added file 'data/img/orange_x.png'
75Binary files data/img/orange_x.png 1970-01-01 00:00:00 +0000 and data/img/orange_x.png 2010-03-19 11:51:34 +0000 differ
76=== added file 'data/img/search.png'
77Binary files data/img/search.png 1970-01-01 00:00:00 +0000 and data/img/search.png 2010-03-19 11:51:34 +0000 differ
78=== added file 'data/img/superfluous-bubbles.png'
79Binary files data/img/superfluous-bubbles.png 1970-01-01 00:00:00 +0000 and data/img/superfluous-bubbles.png 2010-03-19 11:51:34 +0000 differ
80=== added file 'data/in_development.html'
81--- data/in_development.html 1970-01-01 00:00:00 +0000
82+++ data/in_development.html 2010-03-19 11:51:34 +0000
83@@ -0,0 +1,34 @@
84+<!doctype html>
85+<html>
86+<head>
87+<title>Loading error</title>
88+<link rel="stylesheet" href="reset.css" type="text/css" media="all">
89+<link rel="stylesheet" href="screen.css" type="text/css" media="all">
90+
91+</head>
92+<body class="downloads">
93+<div id="header">
94+
95+<div class="wrap">
96+<h1 id="logo">Ubuntu One Music Store</h1>
97+
98+</div><!-- close .wrap -->
99+</div><!-- close #header -->
100+
101+<div class="superfluous">
102+<div id="content">
103+<div class="access">
104+ <h1>In development</h1>
105+<h2>The music store is under development, please be patient</h2>
106+</div>
107+</div>
108+
109+</div>
110+</div>
111+<div id="footer">
112+<div class="wrap">
113+</div>
114+</div>
115+</body>
116+</html>
117+
118
119=== added file 'data/load_error.html'
120--- data/load_error.html 1970-01-01 00:00:00 +0000
121+++ data/load_error.html 2010-03-19 11:51:34 +0000
122@@ -0,0 +1,41 @@
123+<!doctype html>
124+<html>
125+<head>
126+<title>Loading error</title>
127+<link rel="stylesheet" href="reset.css" type="text/css" media="all">
128+<link rel="stylesheet" href="screen.css" type="text/css" media="all">
129+
130+<script>
131+function reload() {
132+ reload_url = location.search.substr(1);
133+ location.href = reload_url;
134+ return false;
135+}
136+</script>
137+</head>
138+<body class="downloads">
139+<div id="header">
140+
141+<div class="wrap">
142+<h1 id="logo">Ubuntu One Music Store</h1>
143+
144+</div><!-- close .wrap -->
145+</div><!-- close #header -->
146+
147+<div class="superfluous">
148+<div id="content">
149+<div class="access">
150+ <h1>Internet connection is required to access the music store</h1>
151+<h2>Please connect and reload.</h2>
152+<p><button onclick="reload()"><span>Reload</span></button></p>
153+</div>
154+</div>
155+
156+</div>
157+</div>
158+<div id="footer">
159+<div class="wrap">
160+</div>
161+</div>
162+</body>
163+</html>
164
165=== added file 'data/reset.css'
166--- data/reset.css 1970-01-01 00:00:00 +0000
167+++ data/reset.css 2010-03-19 11:51:34 +0000
168@@ -0,0 +1,49 @@
169+pan, applet, object, iframe,
170+h1, h2, h3, h4, h5, h6, p, blockquote, pre,
171+a, abbr, acronym, address, big, cite, code,
172+del, dfn, em, font, img, ins, kbd, q, s, samp,
173+small, strike, strong, sub, sup, tt, var,
174+dl, dt, dd, ol, ul, li,
175+fieldset, form, label, legend,
176+table, caption, tbody, tfoot, thead, tr, th, td {
177+ margin: 0;
178+ padding: 0;
179+ border: 0;
180+ outline: 0;
181+ font-weight: inherit;
182+ font-style: inherit;
183+ font-size: 100%;
184+ font-family: inherit;
185+ vertical-align: baseline;
186+}
187+/* remember to define focus styles! */
188+:focus {
189+ outline: 0;
190+}
191+body {
192+ line-height: 1;
193+ color: black;
194+ background: white;
195+}
196+ol, ul {
197+ list-style: none;
198+}
199+/* tables still need 'cellspacing="0"' in the markup */
200+table {
201+ border-collapse: separate;
202+ border-spacing: 0;
203+}
204+caption, th, td {
205+ text-align: left;
206+ font-weight: normal;
207+}
208+blockquote:before, blockquote:after,
209+q:before, q:after {
210+ content: "";
211+}
212+blockquote, q {
213+ quotes: "" "";
214+}
215+
216+strong {font-weight: bold;}
217+em {font-style: italic;}
218
219=== added file 'data/screen.css'
220--- data/screen.css 1970-01-01 00:00:00 +0000
221+++ data/screen.css 2010-03-19 11:51:34 +0000
222@@ -0,0 +1,336 @@
223+/* copied from original html */
224+
225+#downloads {
226+ padding: 0;
227+ margin: 0;
228+}
229+#downloads li {
230+ overflow: hidden; /* enclose floats */
231+ zoom: 1; /* hasLayout in IE */
232+ border-bottom: 1px dotted black;
233+ list-style: none;
234+ padding: 0;
235+ margin: 0;
236+}
237+
238+li .metadata {
239+ float: left;
240+ width: 450px;
241+}
242+
243+.progress .progress-bar {
244+ float: right;
245+ width: 154px;
246+ height: 13px;
247+ background: url(img/progress-bar-background.png) no-repeat left;
248+}
249+
250+.progress .progress-bar .gradient {
251+ height: 13px;
252+ background: url(img/gradient.png) no-repeat left;
253+ -webkit-transition: width 2s linear;
254+ /* 2s should be the same as the Rhythmbox update interval */
255+}
256+.progress .progress-string {
257+ float: right;
258+ clear: right;
259+}
260+
261+.progress .progress-complete {
262+ background: url(img/musicstore-complete-tick.png) no-repeat right;
263+ padding-right: 28px;
264+ -webkit-transition: opacity 0.5s linear;
265+ height: 20px;
266+ padding-top: 10px;
267+ display: inline-block;
268+}
269+
270+/* originating with this file */
271+
272+body {
273+ font-family: "bitstream vera sans", "dejavu sans", verdana, sans-serif;
274+ margin: 0 auto;
275+ font-size: 11px;
276+ line-height: 14px;
277+}
278+
279+.wrap {
280+ margin: 0 auto;
281+ width: 730px;
282+}
283+
284+#header {
285+ background: url(img/header_background.png) repeat-x;
286+ height: 86px;
287+ min-width: 550px;
288+ margin-bottom: 0;
289+ margin-top: -50px;
290+ }
291+
292+#header ul {padding: 0; margin: 0;}
293+#header ul li {display: inline; margin: 0; padding: 0;}
294+
295+#header ul#home_nav {float: left;}
296+
297+ul#home_nav li a {
298+ display: block;
299+ float: left;
300+ text-indent: -9999px;
301+ width: 26px;
302+ height: 41px;
303+}
304+
305+ul#home_nav li.back a {
306+ background: url(img/menu.png) no-repeat;
307+ background-position: 0 0;
308+}
309+
310+ul#home_nav li.back a:hover {
311+ background: url(img/menu.png) no-repeat;
312+ background-position: 0 -41px;
313+
314+}
315+
316+ul#home_nav li.home a {
317+ width: 42px;
318+ background: url(img/menu.png) no-repeat;
319+ background-position: -26px 0;
320+}
321+
322+ul#home_nav li.home a:hover {
323+ width: 42px;
324+ background: url(img/menu.png) no-repeat;
325+ background-position: -26px -41px;
326+}
327+
328+#header ul#nav {
329+ float: right;
330+ background: url(img/menu.png) no-repeat;
331+ background-position: top right;
332+ padding-right: 38px;
333+}
334+
335+#nav li a {
336+ float: left;
337+ display: block;
338+ text-indent: -9999px;
339+ height: 41px;
340+}
341+
342+#nav li.new a {
343+ width: 57px;
344+ background: #000 url(img/menu.png) no-repeat;
345+ background-position: -320px 0;
346+}
347+
348+#nav li.new a:hover {
349+ background: #000 url(img/menu.png) no-repeat;
350+ background-position: -320px -41px;;
351+}
352+
353+#nav li.just_added a {
354+ width: 90px;
355+ background: url(img/menu.png) no-repeat;
356+ background-position: -377px 0;
357+}
358+
359+#nav li.just_added a:hover {
360+ background: url(img/menu.png) no-repeat;
361+ background-position: -377px -41px;
362+}
363+
364+#nav li.browse a {
365+ width: 69px;
366+ background: url(img/menu.png) no-repeat;
367+ background-position: -467px 0;
368+}
369+
370+#nav li.browse a:hover {
371+ background: url(img/menu.png) no-repeat;
372+ background-position: -467px -41px;
373+}
374+
375+#nav li.downloads a {
376+ width: 109px;
377+ background: url(img/menu.png) no-repeat;
378+ background-position: -536px 0;
379+}
380+
381+body.downloads #nav li.downloads a:hover, #nav li.downloads a:hover {
382+ background: url(img/menu.png) no-repeat;
383+ background-position: -536px -41px;
384+}
385+
386+body.downloads #nav li.downloads a {
387+ background: url(img/menu.png) no-repeat;
388+ background-position: -536px -82px;
389+}
390+
391+#nav li.basket a {
392+ width: 65px;
393+ background: url(img/menu.png) no-repeat;
394+ background-position: -645px 0;
395+}
396+
397+#nav li.basket a:hover {
398+ background: url(img/menu.png) no-repeat;
399+ background-position: -645px -41px;
400+}
401+
402+#nav li.help a {
403+ width: 52px;
404+ background: url(img/menu.png) no-repeat;
405+ background-position: -710px 0;
406+ }
407+
408+#nav li.help a:hover {
409+ background: url(img/menu.png) no-repeat;
410+ background-position: -710px -41px;
411+ }
412+
413+.navish {overflow: auto;}
414+
415+h1#logo {
416+ margin-top: 60px;
417+ display: block;
418+ text-indent: -9999px;
419+ background: url(img/logo.png) no-repeat;
420+ width: 190px;
421+ height: 15px;
422+ float: left;
423+}
424+
425+#header form {
426+ float: right;
427+ margin-top: 10px;
428+}
429+
430+#content {
431+ margin: 0 auto;
432+ width: 760px;
433+ background: url(img/content_top.png) no-repeat;
434+ min-height: 300px;
435+ text-align: left;
436+ padding-top: 25px;
437+}
438+
439+.access {
440+ margin: 0 auto;
441+ width: 500px;
442+ margin-top: 130px;
443+ }
444+
445+#content .access h1 {
446+ font-weight: bold;
447+ margin: 0;
448+ font-size: 15px;
449+ margin-bottom: 10px;
450+ }
451+
452+#content .access h2 {
453+ font-weight: normal;
454+ font-size: 13px;
455+ margin: 0;
456+ margin-bottom: 20px;
457+ }
458+
459+#content h2 {
460+ font-size: 20px;
461+ font-weight: normal;
462+ margin: 0;
463+ padding: 0;
464+ text-align: left;
465+ margin-bottom: 25px;
466+ margin-left: 15px;
467+}
468+
469+.progress {
470+ text-align: right;
471+ float: right;
472+ width: 280px;
473+}
474+
475+ul.downloading {
476+ color: #8f8f8f;
477+ border-top: 1px dotted #999999;
478+ border-bottom: 1px dotted #999999;
479+ margin-left: 15px;
480+}
481+
482+ul.downloading li {
483+ border-bottom: 1px dotted #d6d6d6;
484+ padding-top: 20px;
485+ padding-bottom: 20px;
486+ overflow: auto;
487+}
488+
489+ul.downloading li.last {
490+ border-bottom: none;
491+}
492+
493+
494+.superfluous {
495+ background: url(img/downloads_background.png) repeat-x 50% 100%;
496+ padding-bottom: 100px;
497+}
498+
499+
500+#footer {
501+ background: url(img/superfluous-bubbles.png) no-repeat 380px 0%;
502+ margin: 0 auto;
503+ margin-top: -75px;
504+ width: 750px;
505+ padding-top: 100px;
506+}
507+
508+button#music-search {
509+ background: url(img/search.png) no-repeat;
510+ text-indent: -9999px;
511+ width: 77px;
512+ height: 22px;
513+ border: 0;
514+}
515+
516+
517+.progress p, .progress .progress-string {padding-right: 3px;}
518+
519+p.copyright {color: #8f8f8f; margin-bottom: 8px;}
520+
521+#pagination {
522+ overflow: hidden; /* enclose floats */
523+ zoom: 1;
524+}
525+#pagination #count {
526+ float: left;
527+}
528+#pagination #pages {
529+ float: right;
530+}
531+#pagination #pages a {
532+ text-decoration: none;
533+ margin: 4px;
534+ color: #8f8f8f;
535+}
536+#pagination #pages a.current {
537+ color: #f58027;
538+}
539+
540+button {
541+ background: url(img/button-left.png) no-repeat left;
542+ border: 0;
543+ padding: 0;
544+ padding-left: 7px;
545+ display: inline-block;
546+ text-align: center;
547+ }
548+
549+
550+button span {
551+ background: url(img/button-right.png) no-repeat right;
552+ display: block;
553+ white-space: nowrap;
554+ color: white;
555+ font-size: 12px;
556+ padding: 4px 24px 3px 15px;
557+ margin-left: 1px;
558+ }
559
560=== added file 'data/staticstyle.css'
561=== modified file 'libubuntuone/u1-music-store.c'
562--- libubuntuone/u1-music-store.c 2010-03-12 16:12:36 +0000
563+++ libubuntuone/u1-music-store.c 2010-03-19 11:51:34 +0000
564@@ -37,10 +37,10 @@
565 #define U1_NOT_LOGGED_IN_STORE_URL "/music/store-no-token"
566 #define U1_NOT_REGISTERED_URL "https://one.ubuntu.com/music/notregistered?returnUrl="
567
568+#define U1_DEFAULT_ERROR_PAGE "load_error.html"
569+#define U1_IN_DEVELOPMENT_PAGE "in_development.html"
570+#define U1_CONNECTING_PAGE "connecting.html"
571 #define U1_INITIAL_PAGE "<html><body>Loading Ubuntu One music store</body></html>"
572-#define U1_DEFAULT_ERROR_PAGE "<html><body>Could not load Music Store</body></html>"
573-#define U1_IN_DEVELOPMENT_PAGE "<html><body>The music store is under development, please be patient</body></html>"
574-#define U1_CONNECTING_PAGE "<html><body>Connecting to Ubuntu One. Please wait...</body></html>"
575
576 struct _U1MusicStorePrivate {
577 DBusGConnection *bus;
578@@ -151,6 +151,24 @@
579 object_class->finalize = u1_music_store_finalize;
580 }
581
582+
583+static void
584+load_internal_html_page (WebKitWebView *web_view, const gchar *file_name, gchar *reload_url)
585+{
586+ gchar *calculated_file_path, *calculated_file_url;
587+ calculated_file_path = g_build_path ("/", U1_JAVASCRIPT_DIR, file_name, NULL);
588+ if (reload_url == NULL) {
589+ calculated_file_url = g_filename_to_uri(calculated_file_path, NULL, NULL);
590+ } else {
591+ calculated_file_url = g_strdup_printf("%s?%s",
592+ g_filename_to_uri(calculated_file_path, NULL, NULL),
593+ reload_url);
594+ }
595+ webkit_web_view_open (web_view, calculated_file_url);
596+ g_free(calculated_file_url);
597+ g_free(calculated_file_path);
598+}
599+
600 static void
601 parse_oauth_string (const gchar *string, gchar **oauth_token, gchar **oauth_token_secret)
602 {
603@@ -319,7 +337,7 @@
604 gboolean success;
605 GError *error = NULL;
606
607- webkit_web_view_load_string (web_view, U1_CONNECTING_PAGE, "text/html", "utf-8", "file:///");
608+ load_internal_html_page(web_view, U1_CONNECTING_PAGE, NULL);
609
610 proxy = dbus_g_proxy_new_for_name (music_store->priv->bus, "com.ubuntuone.Authentication",
611 "/", "com.ubuntuone.Authentication");
612@@ -648,8 +666,7 @@
613 load_error_cb (WebKitWebView *web_view, WebKitWebFrame *frame, const gchar *uri, GError *error, gpointer user_data)
614 {
615 U1MusicStore *music_store = U1_MUSIC_STORE (user_data);
616-
617- webkit_web_view_load_string (web_view, U1_DEFAULT_ERROR_PAGE, "text/html", "utf-8", "file:///");
618+ load_internal_html_page(web_view, U1_DEFAULT_ERROR_PAGE, uri);
619
620 return TRUE;
621 }

Subscribers

People subscribed via source and target branches