Closed slayerofdrosophila closed 2 years ago
Hi @slayerofdrosophila! Thank you for this PR.
Because this fix was something our team wanted to get done as part of the current sprint, I've gone ahead and opened a separate PR for this issue (PR #321 )
Your PR was very good. The comments I'd have is that we should use react-router's <Link>
tag since this would allow the component to render as an anchor tag (<a>
) which lets us take advantage of the browser's builtin link behavior (such as ability to tab and access links with "Enter" key). The <Link>
component also lets us use react-router's push history, so we don't have to force a refresh.
I encourage you to keep contributing to Scout! This PR came in during a busier time, hence my delay, but I should be able to review any future PRs faster.
Summary
I don't think this change is correct because the linter made changes to my working copy which I didn't include here, one of which was deleting the App.jsx file. However, it did work locally before I ran git commit.
Please do not merge this; I am trying to learn how to develop for Scout.
Screenshots or Videos (if applicable)
It behaves the same as before, but if you click on the card itself, it also brings you to the collection page. Back button works still.
Related Issues
Related to #259. Thank you for listing "good first issue" issues.
Test Plan
Checklist Before Requesting a Review