Early Tutors: Initial pass at a new tutorial living in the repo #1846
Conversation
| def sendMessage = { | ||
| var message = ChatMessage("", "") | ||
|
|
||
| "#new-message" #> SHtml.text(message, { body: String => message = ChatMessage(username.is, body) }) & |
There was a problem hiding this comment.
Some newbie questions that this line gave for me:
-
What is this curly bracket thing? - is it an anonymous function definition in Scala?
textexpects a function as its second parameter, so maybeString => messageis the function type. Butmessageisn't a type, hmm. No, wait, aha -bodyis a self type in an anonymous class and the mutable variablemessageis set to a new ChatMessage instance - or? Having it on one line is really confusing. Do we need an anonymous class with a String self type? -
What is this method
isonusername? I pasted the code into a Lift project and realised it was simply a getter method on the username session variable to retrieve the value, duh. There's also a shadow methodgetthat points to theismethod. For a newbie tutorial, I would suggest to simply use thegetmethod instead. For seasoned Lift users theismethod is probably obvious butgetis more generally understood when coming from the outside (as the audience of a tutorial would likely be).
Since message is now a ChatMessage, we need to say message.body for the first argument to text, no?
Could we instead say something like this?
"#new-message" #> SHtml.text(message.body, newMessage => { message = ChatMessage(username.get, newMessage) }) &It would be great to describe the line in a similar manner as you so well described the previous lines in part 5:
"#new-message" #> SHtml.text(message, message = _) &
"type=submit" #> SHtml.submitButton(() => {
messageEntries ::= message
})Once understanding passing NodeSes => NodeSeq functions around, css-selectors become more natural. So I think it is worth explaining examples of those thoroughly.
There was a problem hiding this comment.
These are great points. A couple of responses:
- One of the lines I try to walk here is making sure that I don't stray too far into explaining the basics of Scala. I don't think this tutorial is best served by trying to teach both Scala and Lift. That said, I do dabble very slightly in that in a few places, and I think it's absolutely a good idea to break this one down a little more.
- I'm not using the
thing => { body }style because I'm not a fan :)
Pushed some updates that hopefully address some of this, let me know!
There was a problem hiding this comment.
Hah! I actually completely fumbled this one, updating the bit further down and forgetting this start part was here. That's what I get for trying to do it quickly and while tired. I'll do another pass tomorrow to clean things up :)
We now use asciidoc callouts to more clearly link back to particular parts of the code that was updated, and break it down into two more digestible chunks.
|
Okay, just completely reworked that whole hunk, and started using asciidoc code callouts to boot… I think it's looking a lot better, let me know if you agree @marcgrue ! |
|
Excellent, Antonio! Clear description that is very helpful. I'm very happy :-) |
|
Awesome! Good to hear :) Thanks for looking through the content! |
| interface from the process of putting data from the system into it, in a way | ||
| that lets you stay focused on users when you're creating the user interface and | ||
| worry about the the interface between your backend and the HTML only when | ||
| you're working on the backend. |
There was a problem hiding this comment.
This might be easier to parse if it were broken into two sentences right around in a way
| you're working on the backend. | ||
|
|
||
| The flip side of view-first development is that it takes some getting used to | ||
| if one is accustomed to the typical web MVC framework. The first stop when |
There was a problem hiding this comment.
if one is accustomed — I think we should feel free to use the second-person pronouns where applicable. We do so in the first paragraph, and it makes the sentence feel more personal and conversational to me to read if you're accustomed. Using one has more of an academic, aloof texture.
There was a problem hiding this comment.
I prefer no contractions, since this is written, but I'm down to go second-person to make it less academic.
| if one is accustomed to the typical web MVC framework. The first stop when | ||
| figuring out what's going on in a typical web MVC setup is the controller. In | ||
| Lift, your first stop is your HTML file. Everything starts in the HTML, and in | ||
| what it is that you want to present to the user. You don't just think about |
There was a problem hiding this comment.
Everything starts in the HTML, and in what it is that you want to present to the user.
This sentence took me a few reads to comprehend correctly. It might be a good candidate for some tweaking.
There was a problem hiding this comment.
Rephrased, great point.
| what it is that you want to present to the user. You don't just think about | ||
| user interactions first, you *build* them first, and let them guide your | ||
| development forward and inform it at every step of the way. Turning a usability | ||
| tested high fidelity mockup into a live page has never been so straightforward. |
There was a problem hiding this comment.
Comma after tested for clarity since you're chaining adjectives.
| smart enough to strip all HTML comments in production mode.]. | ||
|
|
||
| When it comes to user testing, notice that our view is fully-valid HTML, with | ||
| placeholder data. It is, in effect, a high-fidelity mockup. And now that we've |
There was a problem hiding this comment.
Here you use high-fidelity with a hyphen. Earlier you use it without. I think we should use the hyphen version consistently.
There was a problem hiding this comment.
Your eye for detail brings a tear to my eye ;)
| Another distinguishing characteristic of Lift is that it is *secure by | ||
| default*. Amongst other things, this means that you can't access a file in your | ||
| `src/main/webapp` directory through your application unless you explicitly | ||
| define that it's meant to be accessed. You define this using Lift's menu |
There was a problem hiding this comment.
This isn't entirely true. If a SiteMap isn't defined at all, then we default to permitting access to all views under the webapp directory that aren't stored in a folder starting with _. Unless, of course, that's changed in a recent commit and I missed it. =)
There was a problem hiding this comment.
! Nice catch. Rephrased a bit to take that into account.
|
|
||
| Snippets let you refer to a block of code that is responsible for rendering a | ||
| particular part of your page. You add these references by augmenting your HTML | ||
| with a few completely valid `data-` attributes that get stripped before the |
There was a problem hiding this comment.
Would a link to the HTML5 specification or some other explanation for data attributes be useful here?
| another way that manifests is that the template HTML is turned into a | ||
| first-class XML tree early in the processing cycle, and snippets just transform | ||
| that tree. That means script injection and a variety of other attacks are | ||
| significantly more difficult against a Lift codebase.]. |
There was a problem hiding this comment.
I'm torn on whether or not the contents of this footnote should be part of the regular paragraph. No further comment than that, just wanted to bring it to your attention.
There was a problem hiding this comment.
Because this is a tutorial, I decided I wanted to keep the top-level content super-focused on implementation rather than reasoning.
|
|
||
| Let's look at our chat app specifically. We're going to bind two things: the | ||
| list of chat messages, and the text input that lets us actually chat. To the | ||
| `ol` that contains the chat messages, we add: |
There was a problem hiding this comment.
The original markup was two pages back. Perhaps we should reproduce it here so the reader is reminded of the overall structure?
There was a problem hiding this comment.
Yes… But let me punt on that a bit. I'm wanting to do a little preprocessing pass on these guys that handles building some of that for you.
| <form class="send-message" data-lift="Chat.sendMessage"> | ||
| ``` | ||
|
|
||
| These two indicate two methods in a class called `Chat`, which Lift searches |
There was a problem hiding this comment.
"These two" -> "These two changes add data attributes that..."
| they're easy to pattern match (as you can see, message handling is done via | ||
| pattern matching footnote:[Strictly speaking, `messageHandler` is a | ||
| `PartialFunction`. This means that it can match any subset of objects that it | ||
| wants to.]) and because they're generally immutable, so there's no chance of |
There was a problem hiding this comment.
Technically, there's little change of someone else trying to modify the message as we're processing it. But, eh, technicalities. heh.
There was a problem hiding this comment.
Yeah, I think that would actually obscure rather than illuminate here.
| on the `Box`. We do this because the type of `messageEntries` is now a | ||
| `Box[List[ChatMessage]]`, meaning a box that *contains* a list of chat | ||
| messages. `flatten` will give us the plain list of messages if the `Box` is | ||
| `Full`, and an empty list if it's `Empty`, which is perfect. |
There was a problem hiding this comment.
Are you sure about that?
@ Box(List("abc")).flatten
cmd2.sc:1: Cannot prove that String <:< net.liftweb.common.Box[B].
val res2 = Box(List("abc")).flatten
^
Compilation FailedThere was a problem hiding this comment.
I am… Not, all of a sudden, lol. Reworking to use openOr, which is a better pattern in this case anyway I think.
| false. Using `Box.asA` ensures that, if someone changes the `ChatActor` later | ||
| to reply with something else, our snippet won't blow up in the user's face—it | ||
| will just not display the existing messages. The intrepid reader can then go | ||
| and fix the issue. |
There was a problem hiding this comment.
It's probably worth noting that someone should probably have unit tests on that actor to ensure people don't just go changing its contract all willy nilly 😉
There was a problem hiding this comment.
True, though even so you can imagine someone changing the test + actor and missing the snippet because the compiler can't complain about it.
| ``` | ||
|
|
||
| The ideal way for this to work would be for you to be able to change the value | ||
| of the field, and have it save. We can do exactly that using Lift's `ajaxText` |
There was a problem hiding this comment.
Not sure about the comma after field. Might also be worthwhile to make mention that the save occurs on blur?
|
|
||
| Our first move will be to change how exactly we handle binding chat messages. | ||
| First, we'll do a quick conversion that puts everything in a `CometActor`, but | ||
| doesn't add any additional functionality. Instead of calling |
There was a problem hiding this comment.
Lawl yeah this guy is… Incomplete 🎉
|
🎵 I have not forgotten youuuuu 🎵 But I'll be trying to wrap up #1876 before circling back here. |
|
This PR is... |
This sets us up for some nice preprocessing later to fill in context around our example blocks.
|
Ok, pushed a variety of changes motivated by the above comments 🎉 I would say we can probably merge with these changes and then iterate on additional tweaks, but if you have time to give it a second read-through or at least look at the tweaks, @farmdawgnation, I won't complain :) |
|
Nah, I say we go ahead and ship this. We don't benefit much from the long lived branch and we benefit a lot from these being in |
Comments/questions/remarks welcome. For comments on specific bits of the documents, ideally leave a line comment in the appropriate part of the diff :)