Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mostly typo fixes and standardization of Chapter # format. #24

Open
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

alexandermorgan
Copy link

@alexandermorgan alexandermorgan commented Jan 6, 2025

Many thanks for this wonderful book! I wanted to show my appreciation with a simple pr with mostly typo fixes. Each commit is pretty atomic just fixing one thing and the commit message says what the change is. The only systematic change is that the different ways chapters are referenced (Chapter 3, chapter 3, Chapter Three) are normalized to Chapter 3.

There are some other things I noticed that I didn't change in this pr, either because I don't know how to or because it's more debatable. I'll share those things here for your consideration:

  1. In Chapter 6, your backend code has a list comprehension followed by a regular for loop. The for loop syntax inside of a list comprehension is kind of backwards with respect to regular for-loop syntax, so it would be better to combine these two loops into one. That would simplify the syntax for readers that are not familiar with python. This is what the result would be:
@app.route("/contacts/", methods=["DELETE"]) <1>
def contacts_delete_all():
    for contact_id in request.form.getlist("selected_contact_ids"): <3>
        contact_id_int = int(contact_id) <2>
        contact = Contact.find(contact_id_int)
        contact.delete() <4>
    flash("Deleted Contacts!") <5>
    contacts_set = Contact.all()
    return render_template("index.html", contacts=contacts_set)

This would also require revisiting the <2> and <3> references in the passage above, as well as the reference to "16 lines of backend code" in Chapter 7. Oh and I noticed that your contact-app repo similarly has two loops but that one uses list(map(.... That's not better than a single for loop either.
2. The urls in the attribution parameters of blockquotes don't parse as links which seems inconsistent for a hypermedia book. E.g. the Ted Nelson quote in Chapter 1.
3. It would be really slick if the footnotes (like those in Chapter 9) displayed the text of the footnote in the way most html elements do with the "title" text. I couldn't tell how to make that work in your setup though.
4. Not a big deal, but the "Apply" button in your "Customize Colors" panel doesn't do anything. Colors get applied as soon as they're selected and pushing the Apply button doesn't even close the panel.
5. Also regarding the color panel, maybe there should be a color picker for the HTML Notes panel background as well. Because if you invert the colors with white text on a black background, the background of those panels doesn't change and white on pale blue is really hard to read.
6. In Chapter 12, the screenshot referred to here seems to have been taken later in the development process. At this point in the book, I don't think we should have the "Edit" button yet. See: #figure([#image("images/screenshot_hyperview_detail_cropped.png")]
7. Also in Chapter 12 you say "the Hyperview client only understands GET and POST." No mistake per se, but I can't believe it. Did you really limit Hyperview to only GET and POST requests? Why would that be the case?

Copy link

netlify bot commented Jan 6, 2025

Deploy Preview for magenta-kringle-251500 ready!

Name Link
🔨 Latest commit 0aba0d2
🔍 Latest deploy log https://app.netlify.com/sites/magenta-kringle-251500/deploys/677c0b0a02a5b6000850509b
😎 Deploy Preview https://deploy-preview-24--magenta-kringle-251500.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 6, 2025

Deploy Preview for hypermedia-systems ready!

Name Link
🔨 Latest commit 0aba0d2
🔍 Latest deploy log https://app.netlify.com/sites/hypermedia-systems/deploys/677c0b0adc5b9200084cd4f5
😎 Deploy Preview https://deploy-preview-24--hypermedia-systems.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant