WIP: Add Links in EmotionResults to correct Learn page; fix antipattern in Play - closes #31 and #36 #40
Conversation
letakeane
changed the title
|
Looks pretty good Leta. I might have missed something, but why are you filtering out contempt? If you're not using it at all, it could potentially be easier to filter it out right at the response from the api call. |
| ) | ||
| } | ||
| } | ||
|
|
||
| const showResults = () => { | ||
| let emotionsForDisplay = analyzeResponse(results); |
There was a problem hiding this comment.
I could be wrong but I think this should be a const. I don't think you're reassigning it.
There was a problem hiding this comment.
The results change, so the output of analyzeResponse changes ... but does that matter? I guess each time it's a new instance of emotionsForDisplay. I'm not mutating it once it's set.
|
First of all the app looks incredible, it's a very clean UI and really cool feature. I think this is a great addition and adds another layer from the user can learn. Nice job. Is the info in the learn more feature also coming back from the api? |
|
@jbevis No, haha, I had to write it myself. I need to add references and further reading, still, but that's not MVP. I had a minor existential crisis when I was writing the happiness page - "What IS happiness?!" |
What's this PR do?
I added in
<Link>tags to the EmotionResults component so users can learn more about the emotions they are displaying.I intentionally did not create a link for "neutral".
I also changed the antipattern of using
function.bind(this)in the props to a dumb component.Learning
Where should the reviewer start?
Take a look at the screenshot and let me know if the flow makes sense.
Any background context you want to provide?
I'm also trying to hook up a button for the intended emotion - the one selected on the left-hand side of the screen.
Screenshots
Demo of the learn-more link:
