bonham000 / fcc-react-tests-module

The original freeCodeCamp React/Redux alpha curriculum.
http://hysterical-amusement.surge.sh/
BSD 3-Clause "New" or "Revised" License
77 stars 38 forks source link

QA: General Feedback #17

Closed bonham000 closed 7 years ago

bonham000 commented 7 years ago

This Issue is the place to discuss any general feedback on these challenges.

alayek commented 7 years ago

One of my pet peeve was the gazillions of live rendering errors in console. I hope this would go away in the main site, right?

Otherwise, students would have a hard time trying out some snippets in browser console, while working on these.

bonham000 commented 7 years ago

Yeah I agree and yes we could remove these here but I've left them because they have helped us developing and debugging some of the challenges. They shouldn't be in the final version.

dhcodes commented 7 years ago

cmd + enter to submit a challenge seems a bit 'Mac-centric' since windows and linux machines don't usually have a cmd key. Should we list an alternative for other OS?

bonham000 commented 7 years ago

Let me take a look at implementing ctrl + enter for non-Mac machines as a way to test the challenge code if that is a good key combination for this?

dhcodes commented 7 years ago

I think that's what FCC uses per this thread https://github.com/FreeCodeCamp/FreeCodeCamp/issues/9850

bonham000 commented 7 years ago

Okay I'm going to go ahead and add this. Should be available in the next few minutes.

ghost commented 7 years ago

I think we should have the Live Preview below the code, rather than above. When the live preview updates, it's easy to lose track of where you are in your code.

ProfessorSalty commented 7 years ago

Since I've only been posting issues about the problems I've found, I'd like to say that this has been one of the best introductions to React that I've ever seen. This is a fantastic course that's going to help a lot of people.

ethanrose commented 7 years ago

I started from Redux 1, and I'm on lesson 12 now.. It's super awesome and clear, and I'm learning a lot! Thank you. I'm just making pull requests for typos -- everything seems to be working perfectly for me so far.

ethanrose commented 7 years ago

So in redux lesson 13, none of the tests seem to be working, even the console output doesn't show anything when I console.log() it's the "write a counter in redux" - here's my code

const INCREMENT = 'plus';
const DECREMENT = 'minus';

const counterReducer = (state = 0, action)=>{
    switch(action.type){
    case INCREMENT:
      return state + 1;
    case DECREMENT:
      return state - 1;
    default:
      return state;
}

const incAction = {type: INCREMENT}

const decAction = {type: DECREMENT}

const store = Redux.createStore(counterReducer)

console.log('test')
bonham000 commented 7 years ago

I think it's probably functioning correctly. There needs to be a closing bracket } to end the switch statement and both action creators incAction and decAction have to be functions that return objects with a type property. Otherwise, this code looks good and passes when I test it with those modifications.

ethanrose commented 7 years ago

Ah my bad! You're right, that works.

bonham000 commented 7 years ago

No worries! By the way thanks a ton for working on the Redux challenges and sending in that PR. I'm about to merge it. Haven't gotten a ton of feedback on the Redux challenges yet so I'm glad to hear you like them. πŸ‘

tommygebru commented 7 years ago

Update css stylesheet to include this declaration body{width:100vw;}

bonham000 commented 7 years ago

What issue are you seeing with the width?

tommygebru commented 7 years ago

Fix a typo, "consecutively"

BWilkey commented 7 years ago

I just wanted to thank you for your work on this. I've been looking forward to working on React, and these challenges are great. I've only made it through the first 24 so far, but I'll keep going and send in any thoughts.

bonham000 commented 7 years ago

Awesome! Thanks a lot for your feedback it's really helpful!

bonham000 commented 7 years ago

I've updated the code with the recent changes you've suggested but it won't show up in the live version immediately, FYI.

tommygebru commented 7 years ago

No worries the experience is pretty awesome so far

Jesse989 commented 7 years ago

In number 28 only the first two functions are shown as green in the editor, and the following ones are white which might be confusing because that could mean there is a syntax error that is keeping them from being seen as functions.

bonham000 commented 7 years ago

I agree and an issue like this was brought up previously. It probably is due to the way the code editor component is trying to syntax highlight JSX... I'll keep on an eye on this and if it is still an issue once the challenges are merged into the core FCC curriculum we may try to look into it more closely and fix it.

Jesse989 commented 7 years ago

I found another small thing, I hope its not being to nit-picky. In React 43 the code editor has a comment // change code below this line in the parent component, but when I did the exercise and then when I looked at the solution, the code below that line was the same. Could be confusing I think.

bonham000 commented 7 years ago

You're right about that, that's a good thing to fix. I've just changed it, thanks!

Jesse989 commented 7 years ago

On React 46, it says put "Eat, Code, Sleep, Repeat" into the input field, but anytime you push Shift+S it changes the code to the Solution Code.

edit: just realized this does not matter because when it is on freecodecamp.com it will not have the solution.

bonham000 commented 7 years ago

Yes, that's because we made that a shortcut key to load the solution code... So it will get in the way if you are trying to type a capital S with the shift key. This was mainly for use when we were developing this so it won't be included in the final version. Shift R reloads the seed code.

juandaco commented 7 years ago

Hi. Thank you for your work on this . I found a very minor spelling issue in React 07. In Instructions the myComponent starts with minuscule when it should be MyComponent.

bonham000 commented 7 years ago

@juandaco thanks! Got it changed.

Jesse989 commented 7 years ago

Just found another really minor thing that would be fine as is, but would be more consistent if it were changed. On React/Redux 03 it says to call the reducer "messageReducer()" but in the comments it says to create "reducer()". The only reason I bring it up is because the test is looking for a specific function name.

I also wanted to say how excited I am to get to work through these challenges, you have done a really great job! By far the most valuable React/Redux learning resource I have seen, and I spent a lot of time looking at so many tutorials. Thank you!

bonham000 commented 7 years ago

Ok, @Jesse989 that's a good point and I've fixed it now. And thanks for the positive comments!

dgrcode commented 7 years ago

Hi guys, I want to thank you for all the work and effort put together to create this awesome learning material. I'm going through the challenges and so far reached number 8. I currently know some React that I had to learn in other tutorials over the internet, but I'll do them all to give some feedback. I will join the people saying that this is the best material to learn react that I've found on the internet so far. As a bonus I will learn Redux here, that I haven't learn yet :smiley:

I have found a bit confusing one sentence at challenge 2. I must say that English is not my mother tongue, and that could be the reason behind my confusion. The intro says (italics for the confusing sentence):

Intro: The last challenge was a simple example of JSX, but it can include complex nested HTML as well. One important thing to know about nested JSX is that it ultimately needs to return only one outer element. This single parent element would wrap all of the other levels of nested elements. The JSX will not transpile if, for example, your code has several elements that are siblings, but no parent element wrapping all of them. Here's an example:

I would use there something more straight like: The JSX will not transpile if there are more than one element at the root level, without a parent element wrapping all of them. For me, the "for example" bit was the confusing part, but as I said it might be just the way I understood it.

I'll keep writing here any feedback that I have. And thank you for your work again :clap:

bonham000 commented 7 years ago

Great, thanks @dgrcode! I just made some edits to that challenge. If you have any more feedback later on feel free to open another Issue specifically for that challenge, thank you!

dgrcode commented 7 years ago

I've seen the edits. Crystal clear now! Just to mention that there's a small misspelling on the edit: "... will not transpilte"

bonham000 commented 7 years ago

Oh, oops someone submitted a PR for that a few days ago but I hadn't updated it yet. I have now, it should all be correct!

Y-Taras commented 7 years ago

in Firefox buttons do not look as it should.

bonham000 commented 7 years ago

@Y-Taras, you're thanks for noticing. I just adjusted the CSS to fix this.

jayullman commented 7 years ago

I just finished all of the React challenges (1-48). After trying several different React tutorials from various sources, this is the one that really solidified a lot of the confusing pieces for me. Thanks for all of your hard work on this!

bonham000 commented 7 years ago

@libeja thanks for your comments! I really appreciate the feedback. I hope you continue through to the Redux challenges as well. Although these are shorter than the React challenges we've gotten much less feedback on these so I'm really keen to hear what people think about them. Thanks again!

AdventureBear commented 7 years ago

I am a React newbie and couldn't even tackle this until i'd familiarized myself with React somewhat. I'm 4 hours into a front end masters React course as my first exposure. Looking at these challenges I am incredibly impressed with the work that has gone into it.

My initial feedback is that the jump from challenge 7 to challenge 8 seems huge to me...it's about 3 new concepts added at once (super class, props, Kittens, what?). challenges 1-7 seem pretty straightforward for a newbie so far.

dhcodes commented 7 years ago

Below is my general feedback so far. Forgive me if any of these have been said already. Should I make new issues regarding my suggestions for individual challenges or post those here as well?

General Feedback: So far, as many have said I think this is the best React tutorial out right now. I love the attention to detail and your desire to cover everything.

  1. I'm sure someone has mentioned this, but the page is not responsive and FCC has a lot of campers who use mobile devices to do challenges.

  2. Pages should jump back to top when new challenge is loaded. I know there is some debate on this for users who want to only do the challenges based on the tests, but I would guess the vast majority of users want to read the intro, which currently requires scrolling up on load of each new challenge.

  3. I would change the 'Reload Seed' button to 'Reset Code' to keep a similar naming practice to FCC.

  4. Consider side-by-side editor/renderer view so page does not move as changes are made.

  5. Add linting to the editor as a typo will prevent tests from running with no indication of what's wrong.

  6. Hitting the "back" button on the browser will dump you out of the challenges. Would React router help remedy this?

  7. You might consider using a side-by-side approach like the FCC challenges do or move the rendered view below the editor. The reasoning is that as the renderer changes, the editor shifts down the screen as a result.

  8. If you Ctrl + Z or Undo too many times, it will start bringing in code from past challenges.

  9. Is it possible to have the new lines in the editor be pre-indented for where the code should go? This is a silly nitpick but would save some effort for the user and promote code readability.

bonham000 commented 7 years ago

@dhcodes these are all great points and observations! Thanks for your comments. For the time being, because the goal is for all of this content to be ported into the FCC platform when the QA process is finished, I think it's safe to leave most of these issues unaddressed in the app running on Surge. If any of these are still a concern once the material has been moved into FCC we can definitely revisit it.

dhcodes commented 7 years ago

Guess I should read better ☺️. Didn't realize they'd be ported over.

bonham000 commented 7 years ago

No worries!

HKuz commented 7 years ago

@bonham000 and others - here's a point for discussion for maybe adding a new challenge. I noticed in React challenge 46 Use Array.map() to Dynamically Render Elements, the solution added a key to each list item generated by the map callback. It wasn't required to pass the tests, so I didn't think it was worth adding more explanation to that challenge. That said, is this topic (making sure dynamically rendered elements have unique ids so React can track them) worth adding a challenge to cover?

If so, I think we can just re-use a lot of React-46. I can draft up the challenge text, we can use the same example and code but just have the camper include a key, and add one more test to check for keys on the li's. Then we just need to think of one more challenge to make React an even 50 (just kidding πŸ˜„).

bonham000 commented 7 years ago

Actually, yes @HKuz this topic is pretty important and should definitely be included. I thought we had mentioned somewhere about the need to add unique a key for sibling child elements but having looked through the challenges just now it looks like we did not... maybe it was removed somewhere.

Because this is a pretty important thing to include and because Challenge 46 is already so long, I think we should create a Challenge 47 specifically to deal with this. It can be as simple as explaining why we need to create a unique key and then having the user add a unique key attribute to a some data that we are already mapping over. What do you think? If you want to try and write this, that would be great!

And believe me... I'd love to get up to that big 50.

I've just updated the challenge map to reflect this... let me see if I can find any gaps for a 50th challenge. We could add a challenge in the advanced rendering section, maybe after 45, that has you change CSS conditionally, as this is something I've used before and it's not really covered elsewhere. Let me know what you think about this, at the moment no other React topics are coming to mind that we could possibly add.

HKuz commented 7 years ago

Sounds like a plan, @bonham000 - I'll take a shot drafting the new challenge, and will include what you mentioned.

As for that elusive 50th challenge, I'll try to think of anything, too. A challenge on conditional CSS could be a good one!

bonham000 commented 7 years ago

Ok @HKuz we will keep thinking about 50. For now, feel free to go ahead and create 47, you can add the new file and modify App.js to import it in the app but you'll have to modify the file names after it. If you feel comfortable with all that go ahead! Otherwise, you could just create the file and give it some other name for now, e.g. Draft_48.js.

HKuz commented 7 years ago

Okay, thanks for the head's up - I'll give it a shot and will hit you up on Gitter if I run into any issues.

sabrawer commented 7 years ago

Please see these links to the forum, where I put my initial comments. This is a wonderful tutorial - the best out there that I have seen.

https://forum.freecodecamp.com/t/react-tutorial-on-code-pen/74662 https://forum.freecodecamp.com/t/react-tutorial-cokmments-lessons-4-7/74678/3

nickpapasavvas commented 7 years ago

Hello guys and congratulations for the awesome work. I don't know if this is how React works or it is a minor bug but in React Challenge 20 if you give to the key name the value CamperBot inside double quotes like this {name:"CamperBot"} you get an error on the fourth test. So to pass the test the only acceptable is {name:'CamperBot'} (single quotes).