The original wait screen markup was set as same level as view panels, which might need duplication for each views temporary. We might need to more discussion for proper way of reusing the shared widget like wait screen.

Comment on attachment 8596523 [details] [gaia] steveck-chung:new-message-waitscreen > mozilla-b2g:master Hey, It's just a quick experiment patch for testing the html import and append/remove the screen dynamically. It may bring cleaner view markup but not sure if it will increase load time or other penalty, I'll do some profiling for this and any thought is welcome!

Comment on attachment 8596523 [details] [gaia] steveck-chung:new-message-waitscreen > mozilla-b2g:master rel=import for sure competes at start time, and I think it delays the "onload". But that's not the main issue we have here. The main issues are: * HTML Imports is deprecated * Gaia's build system breaks them (see bug 1099185). Because of the first point, it's unlikely we'll work hard to fix this bug. I really don't like the way lazyloader works with external HTML imports, it does too much stuff. I'd suggest to write a simple non-fancy external file importer in template.js instead (that would simply import the external file to a string, turn it into a template, so that we can use it as a template like everywhere else in the SMS app).

Comment on attachment 8596523 [details] [gaia] steveck-chung:new-message-waitscreen > mozilla-b2g:master Yeah, it seems that HTML imports are not going to be shipped, so we probably should not rely on it. Regarding to Julien's suggestion - maybe we can leverage Fetch API instead of XHR to load templates? Thanks!

(In reply to Oleg Zasypkin [:azasypkin] from comment #4) > Comment on attachment 8596523 [details] > [gaia] steveck-chung:new-message-waitscreen > mozilla-b2g:master > > Yeah, it seems that HTML imports are not going to be shipped, so we probably > should not rely on it. > > Regarding to Julien's suggestion - maybe we can leverage Fetch API instead > of XHR to load templates? > > Thanks! Fetch API looks much cleanner and simpler than XMLHttpRequest, thanks for the reminder!

I added an utils function for fetch the html content, haven't tried in real case yet. Will it be closer to expected way for importing external html file? BTW I see clock app already use the similar pattern(They used require('text!some.html') for the external html).

Comment on attachment 8599828 [details] [diff] [review] fetch_WIP.patch Review of attachment 8599828 [details] [diff] [review]: ----------------------------------------------------------------- ::: shared/js/template.js @@ +1,5 @@ > /* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- / > /* vim: set shiftwidth=2 tabstop=2 autoindent cindent expandtab: */ > + > +/*global fetch > +*/ https://github.com/jshint/jshint/pull/2355/files ;) @@ +185,5 @@ > + } > + }, function(err) { > + console.error('fetch operation failed: ' + err.message); > + }); > + }; I'm thinking of something like (in pseudo code): Template.fromFile = function(url) { return fetchUrl(url).then(string) => { var template = new Template(); var members = priv.get(template); members.tmpl = string; delete members.idOrNode; return template; }); } This is quite ugly but this makes backward compatibility easier :) Otherwise we could use Template.fromNode and Template.fromId that would be cleaner but would need to change all code using it.

Comment on attachment 8599828 [details] [diff] [review] fetch_WIP.patch Review of attachment 8599828 [details] [diff] [review]: ----------------------------------------------------------------- Yeah, I think Julien's approach is the way to go here, just two ideas: * Template.fromURL sounds better IMO, it doesn't have to be "file" in general understanding; * + some crazy idea on how to avoid manipulating with priv in fromURL (excuse me if it's stupid :)): What if we have "idOrNodeOrArrayBuffer" instead of "idOrNode" as constructor argument in "Template"? Then we can easily distinguish it from "string" id and "node" (buffer isntanceof ArrayBuffer) in "extract" function. It's also easy to get string content from it: String.fromCharCode(...new Uint8Array(buffer)); And in fromURL we just have (simplified): fetch(url).then((response) => response.arrayBuffer()).then((buffer) => new Template(buffer)). I've made some rough perf measurements and don't see any penalty so far.

TBH I'd rather have a Template.fromNode and Template.fromId than adding more "instanceof" based logic :) I don't really want to add more "instanceof" logic because I think this is quite fragile and not so easy to follow in the code. And we're in shared code here (even if it's not _that_ shared).

(In reply to Julien Wajsberg [:julienw] (PTO May 1st) from comment #9) > TBH I'd rather have a Template.fromNode and Template.fromId than adding more > "instanceof" based logic :) > > I don't really want to add more "instanceof" logic because I think this is > quite fragile and not so easy to follow in the code. And we're in shared > code here (even if it's not _that_ shared). Sure it's better, but I thought you wanted to be backward compatible :) In case of "Template.fromNode" or "Template.fromId" Template's constructor signature will likely change, so that it will accept already extracted string or extraction function bound to the "thing" to extract from on first interpolation call.

I wanted to be backward compatible, hence my proposal that is both backward and forward compatible. But I don't really have a strong opinion.

I understand dev might gain some flexibility if Template supported the ArrayBuffer in constoctor, comparing we set the tmpl directly in comment 7. The original extraction from commented content will still work in Oleg's proposal. But I suspect we really need to comment out the content and extract(uncomment) for interpolation if we load an external html, and we have to split the content loading part into another utils function than Template constructor, which I think single Template.fromUrl would be more straightforward. Template.fromUrl in comment 7 is fair enough IMO, but user need to know the the they don't have to comment out content before using the API.

Mass closing of Gaia::SMS bugs. End of an era :(

Mass closing of Gaia::SMS bugs. End of an era :(