-
Notifications
You must be signed in to change notification settings - Fork 67
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
Add new tutorial - "Anatomy of a CID" #359
Conversation
Hi @zebateira! I just worked through this tutorial and it is great! I encountered no issues. I have submitted #361 that changes stuff a bit when I had to read it twice or noticed an inconsistency. On lesson 03, IPLD is mentioned with no further explanation or references. Is this good? The infographics are very nice, but I think they may have accessibility issues for people who suffer from reduced vision, color blindness or blindness. This is probably out of scope for this pull request, but I thought I would mention this. Have a nice day and thanks for this tutorial :- ) |
Merged 👌
Hm, indeed, maybe it would be nice to have a short sentence and reference link to contextualize it better.
The images should have an Thank you again for your contributions 🙏 |
Honestly, I don't really know much about accessibility (will read up, it's important!). When I did my review, at first I didn't rebuild, so the pictures didn't load which made me realise the difference. So that's why I mentioned it |
I see, but in that case you should've seen the alt text displayed instead, no? |
I see it after rebuilding :-) |
Ah ok. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zebateira This is looking great! Thanks so much for pulling it together!
I did submit a commit with some proposed wording changes for clarity and would appreciate you taking a close look to confirm I haven't misinterpreted anything and changed the meaning unintentionally. Please don't hesitate to push back on any edits that don't make sense to you. I also added this tutorial to the resources and text of the Decentralized Data Structures tutorial and adjusted your Lesson 1 text to recommend reviewing the basics there before starting here. And I fixed the shortname display so it will show as "Anatomy of a CID" instead of "Anatomy Of A Cid."
For the most part, I liked the wrong answers you provided for multiple choice questions, but there were a couple spots where they felt a little forced and one I just didn't understand (see the separate in-line comment). When @alanshaw goes through this he may have ideas of common misconceptions that can be substituted in for a couple of these.
Throughout the tutorial, it feels odd to me when there are references to "which encoding was used" instead of "which encoding method" or "which type of encoding." I don't know if that's a genuine concern or is a matter of my technical vocabulary not being up to snuff. I haven't edited these spots because I'm not confident enough to do it, but maybe @alanshaw can weigh in when he reviews this.
It also feels really awkward that we need to keep referring to CIDs of certain versions - essentially "a Version 0 CID" would be "a CIDv0 CID," which is repetitive. Maybe there are spots where we should replace CID with hash to make it smoother. For now I've just replaced "version zero" with "Version 0," etc. Again @alanshaw may have suggestions on how people talk about this in prose without sounding odd. 😂
Re the alt tags for accessibility: a lot of the images we're using here are easier to understand in picture form that they would be in writing, but if something is super brief, like the string form examples in lesson 5, it's possible we could put the whole text (Qm.... Bafy....) in the alt tag so nothing's getting lost. I'd recommend just taking a spin through the full tutorial to ensure anything essential from the content of the pics is either spelled out in the alt tags or covered in the text that follows.
A couple of more logistical issues I see:
The resources page for the tutorial isn't being checked off as complete in the table of contents after I visit it, and I have the same issue with some but not all of the other tutorials on this branch. May be a pre-existing problem based on lesson type?
I don't recall if there was a reason we couldn't use marked
in the multiple choice answers. It would be ideal to allow the use of code blocks with `` here but I quickly tried it and it interpreted them literally as text.
I pushed some edits myself after reviewing your review (accidentally stumbled upon several repeated 'the'
Good question. Encoding refers to the representation of the data, that's why I didn't use "method" or "type, but maybe "type" could be a valid one. @alanshaw ?
Yes, this is much better 👌
We were able to reproduce this for production even. Created issue in #368
Yeah, we should enable this if we can. Issue created here: #364 |
4b4c18d
to
bed6c4c
Compare
Co-Authored-By: Alan Shaw <alan.shaw@protocol.ai>
Just chatted with @alanshaw about this grammatical concern of mine...
He said that for someone technical it is totally fine to refer to "the encoding" as @zebateira did, and also totally fine to use my suggestions of "the encoding method" or "the type of encoding," but since we want to air on the side of accommodating less technical learners, we may as well go with my suggestions that will appear more grammatically correct to the newcomer. :) |
Flagging two suggestions from a Slack convo w/ @mikeal & @ribasushi today:
I haven't had time to go through all of @ribasushi & @johnnymatthews edits yet but really appreciate all the feedback! |
@terichadbourne added support for multiple projects in #396 which should address this partly. Only missing the text I think. |
* fix: mark resources "lesson" as passed on page mount (#375) * fix: mark resources "lesson" as passed on page mount * pass lessonId to countly for resources Co-authored-by: Teri Chadbourne <terichadbourne@users.noreply.github.com> * distinguish btw file upload v code except in icon component * test reset code functionality * BROKEN test all lesson types w/ cheats * fix(content): cryptographic typo (#379) Closes #377 * Update cypress/integration/tutorials.spec.js Co-Authored-By: Zé Bateira <jose.bateira+git@moxy.studio> * Update cypress/integration/tutorials.spec.js Co-Authored-By: Zé Bateira <jose.bateira+git@moxy.studio> * chore(docs): some boilerplate updates with the new options (#376) * chore(docs): some boilerplate updates with the new options * edits to exportable properties documentation * chore(docs): update docs with the role of modules in the option * typo fix in modules description * Update src/tutorials/boilerplates/boilerplate-code.js Co-Authored-By: Zé Bateira <jose.bateira+git@moxy.studio> Co-authored-by: Teri Chadbourne <terichadbourne@users.noreply.github.com> * debug/refactor loop through all lessons * update gitignore of .env files * protect .env * test resetCode only once per lesson type * test landing pages and headers while looping through lessons * test: tutorials page displays all in correct order * test: featured tutorials shown in correct order on homepage * fix toggle and add test for hiding coding tutorials * lengthen cypress timeout * tweak test names * reinstate loop through codeless tutorials * WIP add mult choice test * fix import of lesson.js file * fix test of choice count * WIP test right/wrong answers for mult choice * refactor test descriptions * fix: eslint config for cypress * fix: test text for markdown parsing is present in quiz component * Increase Cypress tests to check reset code and loop through all lesson types (#378) * distinguish btw file upload v code except in icon component * test reset code functionality * BROKEN test all lesson types w/ cheats * Update cypress/integration/tutorials.spec.js Co-Authored-By: Zé Bateira <jose.bateira+git@moxy.studio> * Update cypress/integration/tutorials.spec.js Co-Authored-By: Zé Bateira <jose.bateira+git@moxy.studio> * debug/refactor loop through all lessons * protect .env * test resetCode only once per lesson type * test landing pages and headers while looping through lessons * test: tutorials page displays all in correct order * test: featured tutorials shown in correct order on homepage * fix toggle and add test for hiding coding tutorials * lengthen cypress timeout * tweak test names * reinstate loop through codeless tutorials * refactor to loop through lessons from tutorials util * improve viewSolution test and refactor loop through tutorials * appease linter Co-authored-by: Zé Bateira <jlageb@gmail.com> * incorporate review feedback * add test for progress status (failing correctly) * fix: lessonPassed - update UI with storage * test progress icons Co-authored-by: Zé Bateira <jlageb@gmail.com>
Quick update after a chat with @ribasushi: His suggestion is that in this image, the |
And ideally the bits should also express 32, or I want to stress again that I understand that the schedule is tight, and there is a number of images to fix. As long as the group makes an informed decision to leave things as-is, having understood the mismatch: I am fine with this 😸 |
This is my mistake. It should really be fixed since it's the "anatomy of a CID" tutorial. The length should be 32 ( (Note the first number, 18 is sha2-256) |
FWIW IPLD formats are mentioned quite a bit in the specs and extensively in the current js-ipld API used by js-ipfs. If these are not up to date and it's not a term being used anymore then please forgive me but I knew no better 🙏 |
To sum up everything of the feedback:
As mentioned by @ribasushi these edits can be fixed after we publish, which might not be a bad idea considering the length of this review and timeline for publishing this tutorial. |
The other missing piece in your list is some light reframing to fit the new Multiformats project label. I can take that on while you're out @zebateira. @alanshaw are the "images" in your preso actually image files or are they Google Slides text boxes and arrows and such? Could I snag a copy of the original Google Slides file? Ze has been working from the PDF you shared in the camp repo. |
Co-Authored-By: Johnny <9611008+johnnymatthews@users.noreply.github.com> Co-Authored-By: Peter Rabbitson <ribasushi@leporine.io>
Latest commit updates images as follows based on recommendations from @ribasushi & @alanshaw. Please let me know if anything isn't as intended. |
* feat: support projects * Feat/support multiple projects option 3 (#399) * fix: ui tweak option 1 * fix: ui tweak option 3 Co-authored-by: Zé Bateira <jlageb@gmail.com> * adjust project name placement Co-authored-by: Teri Chadbourne <terichadbourne@users.noreply.github.com>
@mikeal I believe we've now made all the changes necessary to reframe this as a Multiformats tutorial. Would you have a sec to peruse as an end user (which will be much easier than reviewing the full code diff) and let me know if anything strikes you as incorrect or poorly explained? |
@terichadbourne all the images in #359 (comment) are correct, thank you 🎉 |
Shipping it! Please feel free to open issues for any suggested improvements moving forward. Draft of the announcement blog post can be found here: ipfs-inactive/blog#379 |
- gateway redirects to a valid libp2p-key CID - CLI resolves as-is - style: use 'hostname' in places where host header may have explicit port License: MIT Signed-off-by: Marcin Rataj <lidel@lidel.org>
Closes #329
Closes #364