Question link #63

terichadbourne · 2018-10-16T16:53:01Z

Added a message to the bottom of the lesson template that encourages users to submit their questions/feedback on the lesson, linking to a new issue with the lesson number / title / path pre-filled and a body that explains the type of feedback we'd appreciate.

Addresses issues #45 and #42.

- Added link to GitHub repo for those who need help or have suggestions to improve the lessons - TODO: Improve styling

- Build the link to a new issue programmatically, including the lesson name, number, and path in the issue title and some questions we hope the user will answer in the body. - Remove request for user to copy URL, since we now are providing this automagically. - TODO: Not sure how to match max width of lesson box, though the text here is no longer wide enough for this to matter. - TODO: Create a specific GitHub issue template to use for questions originating from this lesson page, so that nicer formatting can be used in the body. (Don't know how to create line breaks in the current parameter format.)

olizilla · 2018-10-16T17:18:56Z

@terichadbourne thanks! Pls can you drop a screenshot of what you expect to see on any PR with a visual change, so when we run it, we know that we're on the same page.

olizilla · 2018-10-16T17:24:25Z

src/components/Lesson.vue

@@ -194,6 +200,10 @@ export default {
    lessonNumber: function () {
      return this.$route.path.slice(this.$route.path.lastIndexOf('/') + 1)
    },
+    issueURL: function () {
+      let Url = "https://github.com/ipfs-shipyard/proto.school/issues/new?labels=question&title=Question+on+Lesson+" + this.lessonNumber + ": " + this.lessonTitle + " (" + this.$route.path + ")&body=Have+a+question+or+suggestion+regarding+a+ProtoSchool+lesson?+Share+it+here!+We've+noted+the+lesson+your+question+refers+to+in+the+title+of+this+issue.+Please+tell+us+what's+confusing+about+this+lesson.+What+additional+context+could+we+provide+to+help+you+succeed?+What+other+feedback+would+you+like+to+share+about+ProtoSchool?"


Cool! I didn't know you could configure github issues from the url!

With + used in the string and to join it together, this would be mildy more readable as a template string like:

Suggested change

let Url = "https://github.com/ipfs-shipyard/proto.school/issues/new?labels=question&title=Question+on+Lesson+" + this.lessonNumber + ": " + this.lessonTitle + " (" + this.$route.path + ")&body=Have+a+question+or+suggestion+regarding+a+ProtoSchool+lesson?+Share+it+here!+We've+noted+the+lesson+your+question+refers+to+in+the+title+of+this+issue.+Please+tell+us+what's+confusing+about+this+lesson.+What+additional+context+could+we+provide+to+help+you+succeed?+What+other+feedback+would+you+like+to+share+about+ProtoSchool?"

let url = `https://github.com/ipfs-shipyard/proto.school/issues/new?labels=question&title=Question+on+Lesson+${this.lessonNumber}: ${this.lessonTitle} (${this.$route.path})&body=Have+a+question+or+suggestion+regarding+a+ProtoSchool+lesson?+Share+it+here!+We've+noted+the+lesson+your+question+refers+to+in+the+title+of+this+issue.+Please+tell+us+what's+confusing+about+this+lesson.+What+additional+context+could+we+provide+to+help+you+succeed?+What+other+feedback+would+you+like+to+share+about+ProtoSchool?"

Also, uppercase first letter for variable names often implies it's a constructor or some global or module, so best to lowercase this one as url

terichadbourne · 2018-10-16T17:54:51Z

Thanks for the great feedback, @olizilla. I'll work on those updates.

Here's the visual change expected, the addition of this sentence/link under the exercise box on every lesson page on the site:

- Use template string for improved readability of issueUrl - Add formatting to issue body - Change `Url` to `url` and `imageURL` to `imageUrl` to meet standards

terichadbourne · 2018-10-16T22:08:57Z

My latest commit to this branch should address all the issues that @olizilla flagged, as well as adding encoded formatting to the body of the new issue.

@mikeal suggested that I create the issueUrl in the data: self object instead of the computed object, but when I try to add it to the list there as issueUrl: <the ridiculously long string>,
the page doesn't render at all, but also produces no errors in Terminal or the console.

If I instead try issueUrl: return <the ridiculously long string >, it says "Failed to compile" in the browser and thinks there's an unexpected token.

Do I need to adjust some this or self references somewhere to make it work from this section of the code? For reference, this is how the variable is used elsewhere in the code:

<div>
        Feeling stuck? We'd love to hear what's confusing so we can improve
        this lesson. Please <a :href="issueUrl">share your questions and feedback</a>.
      </div>

(What's been committed is the working code before attempting this maneuver, not the failed maneuver.)

- Set a max width on the section containing the issue link to ensure it doesn't get wider than the exercise box should we later change its text. - Use <p> tags around this wording.

terichadbourne · 2018-10-16T22:23:22Z

@olizilla I also just implemented the max width on that footnote section as we discussed. Let me know if you had something else in mind.

- Create the issueUrl in `data: self` instead of in `computed` - Although it currently works, I can't figure out any way to access the `lessonNumber` that lives in `computed`, nor to make a functional `lessonNumber` live in `data: self` so I can pull it from there. I had to rebuild the whole thing within the `issueUrl`, so it adds more code to the template string.

olizilla · 2018-10-17T15:44:49Z

I'd just go with the computed property. I don't know vue, but the docs suggest that computed props are the place to things that you want to derive from other props, as we're doing here

https://vuejs.org/v2/guide/computed.html

Resolve merge conflicts, pulling Mikeal's recent changes from the `fix-output` branch that were already merged into `master` branch

- Change success message from "All works!" to "Everything works!" - Addresses issue #65

terichadbourne · 2018-10-23T18:57:40Z

This branch now also addresses issue #65, correcting a grammar issue in success messages.

- Remove comment lines from earlier merge

- Addresses issues #64 & #38, in which the the "Next" button appeared on the last lesson in a series and directed the user to a non-existent subsequent lesson. This is resolved by now showing instead a "More Workshops" button that goes back to the main list of all workshops so the user can choose what they'd like to do next. - In `Lesson.vue`, change the conditionals for success buttons to include an option (when the solution is correct and `last` has a value of ``"true"`) that loads a button with the text "More Workshops," bound to a new function `workshopMenu` that clears the output and directs the user to the main page, rather than incrementing the lesson number. - In Basics lesson 3 and Blog lesson 7 (the last of each series), follow the steps listed below. - WHEN CREATING THE LAST LESSON OF A NEW WORKSHOP: Add `last="true"` to the values being passed to the Lesson component. Use the success message "Great job! You've completed this series of lessons!" or something else that makes it clear this was the last lesson in the series. The button will populate automatically.

terichadbourne · 2018-10-23T19:49:47Z

@olizilla Would you have a minute tomorrow to test this updated question-link branch for me? Two new things to check for:

You should now see the success message "Everything works!" in most lessons (previously "All works!") . addresses issue Correct grammar in blog series success messages #65
In the final lesson of each series (basics 3 and blog 7), when you correctly solve the problem you should see the success message "Great job! You've completed this series of lessons!" and a "More Workshops" button that takes you back to the list of workshops, rather than a "Next" button that takes you to a non-existent subsequent lesson (blank white screen). addresses issues Last lesson in series links to non-existent following lesson #64 & bug: last lesson should not try to link to "next" lesson. #38

- change "ipld" to "ipfs" in `ipfs.dag.put()` - change "array" to "an array"

olizilla

Good stuff. Very minor change suggestions.

olizilla · 2018-10-29T14:44:37Z

src/components/Lesson.vue

+    <section class="indent-1 mb4 mw-900">
+      <div>
+        <p>Feeling stuck? We'd love to hear what's confusing so we can improve
+        this lesson. Please <a :href="issueUrl">share your questions and feedback</a>.</p>


This would be more friendly if it opens github in a new tab, as we don't currently preserve your state across page reloads... right now if you click on it, your answer is lost.

<a :href="issueUrl" target="_blank">

olizilla · 2018-10-29T14:46:21Z

src/lessons/Basics/03.vue

@@ -37,7 +38,7 @@ const validate = async (result, ipfs) => {
  }

  if (result.value === 1 && result.remainderPath === '') {
-    return {success: 'Great job!'}
+    return {success: "Great job! You've completed this series of lessons!"}


minor, but we're using singlequotes ' for strings in this project. The bigger issue is we should have CI checks and code linting as part of the PR process, but that's an issue for another PR!

olizilla · 2018-10-29T14:46:41Z

src/lessons/Blog/07.vue

@@ -170,7 +171,7 @@ const validate = async (result, ipfs) => {
  } catch (err) {
    return {fail: `Your function threw an error: ${err}.`}
  }
-  return {success: 'All works!'}
+  return {success: "Great job! You've completed this series of lessons!"}


single quotes again

terichadbourne · 2018-10-29T14:51:47Z

Great suggestions, @olizilla! I'll fix those items. I really appreciate you clueing me in to the nuances of formatting choices we're trying to be consistent with in the code across the repo.

terichadbourne · 2018-10-29T14:57:02Z

Actually, @olizilla, I just remembered why I used double quotes there, and it's because that string includes an apostrophe in you've. Would you rather I put backticks (template string format) instead of double quotes on the outside of strings when that's the case?

- In `Lesson.vue`, add `target="_blank"` to link so that when a user clicks the link to log an issue about a question they have, their original window stays open and anything they've typed in the exercise box isn't lost.

- Use backticks for template string instead of double quotes when the string needs to include an apostrophe in the success message for lessons Basics 3 and Blog 7

olizilla · 2018-10-29T15:50:48Z

@terichadbourne I don't think we have anything set up to auto deploy this code yet, so it won't appear on the live site until it's manually updated. There is an issue to fix that #18 and another to fix the CI testing setup #71

olizilla · 2018-10-29T15:51:18Z

@mikeal can you take a look at the CI/CD stuff when you get well? also, get well sir!

terichadbourne added 2 commits October 15, 2018 17:04

Add link to file issue if confused by lesson

b328949

- Added link to GitHub repo for those who need help or have suggestions to improve the lessons - TODO: Improve styling

terichadbourne requested review from mikeal and olizilla October 16, 2018 16:53

olizilla reviewed Oct 16, 2018

View reviewed changes

Update caps and use template string

915ccc7

- Use template string for improved readability of issueUrl - Add formatting to issue body - Change `Url` to `url` and `imageURL` to `imageUrl` to meet standards

Set max width on footnote section

93f237a

- Set a max width on the section containing the issue link to ensure it doesn't get wider than the exercise box should we later change its text. - Use <p> tags around this wording.

terichadbourne added 2 commits October 23, 2018 14:38

Merge branch 'master' into question-link

badce54

Resolve merge conflicts, pulling Mikeal's recent changes from the `fix-output` branch that were already merged into `master` branch

Correct grammar in success message

c8b1f58

- Change success message from "All works!" to "Everything works!" - Addresses issue #65

terichadbourne added 2 commits October 23, 2018 15:01

Remove merge comments

aba940f

- Remove comment lines from earlier merge

Correct typos in blog lesson 3

3bd38cf

- change "ipld" to "ipfs" in `ipfs.dag.put()` - change "array" to "an array"

terichadbourne mentioned this pull request Oct 26, 2018

feature: Migrate routing to a "Lesson Route" #12

Closed

olizilla reviewed Oct 29, 2018

View reviewed changes

terichadbourne added 2 commits October 29, 2018 11:02

Open new GitHub issue in new browser window

2792921

- In `Lesson.vue`, add `target="_blank"` to link so that when a user clicks the link to log an issue about a question they have, their original window stays open and anything they've typed in the exercise box isn't lost.

Use backticks instead of double quotes for strings

c50556a

- Use backticks for template string instead of double quotes when the string needs to include an apostrophe in the success message for lessons Basics 3 and Blog 7

olizilla mentioned this pull request Oct 29, 2018

Configure linter and run it on CI #71

Closed

2 tasks

olizilla merged commit d92fcb3 into master Oct 29, 2018

olizilla deleted the question-link branch October 29, 2018 15:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Question link #63

Question link #63

terichadbourne commented Oct 16, 2018

olizilla commented Oct 16, 2018

olizilla Oct 16, 2018 •

edited

terichadbourne commented Oct 16, 2018

terichadbourne commented Oct 16, 2018 •

edited

terichadbourne commented Oct 16, 2018

olizilla commented Oct 17, 2018

terichadbourne commented Oct 23, 2018

terichadbourne commented Oct 23, 2018

olizilla left a comment

olizilla Oct 29, 2018

olizilla Oct 29, 2018

olizilla Oct 29, 2018

terichadbourne commented Oct 29, 2018

terichadbourne commented Oct 29, 2018

olizilla commented Oct 29, 2018

olizilla commented Oct 29, 2018

Question link #63

Question link #63

Conversation

terichadbourne commented Oct 16, 2018

olizilla commented Oct 16, 2018

olizilla Oct 16, 2018 • edited

Choose a reason for hiding this comment

terichadbourne commented Oct 16, 2018

terichadbourne commented Oct 16, 2018 • edited

terichadbourne commented Oct 16, 2018

olizilla commented Oct 17, 2018

terichadbourne commented Oct 23, 2018

terichadbourne commented Oct 23, 2018

olizilla left a comment

Choose a reason for hiding this comment

olizilla Oct 29, 2018

Choose a reason for hiding this comment

olizilla Oct 29, 2018

Choose a reason for hiding this comment

olizilla Oct 29, 2018

Choose a reason for hiding this comment

terichadbourne commented Oct 29, 2018

terichadbourne commented Oct 29, 2018

olizilla commented Oct 29, 2018

olizilla commented Oct 29, 2018

olizilla Oct 16, 2018 •

edited

terichadbourne commented Oct 16, 2018 •

edited