Refactor Tools data for open-community-survey.md#5096
Refactor Tools data for open-community-survey.md#5096t-will-gillis merged 2 commits intohackforla:gh-pagesfrom yffu:refactor-tools-data-4811
Conversation
… so it displays correctly on all projects pages for the community survey card
|
Want to review this pull request? Take a look at this documentation for a step by step guide! From your project repository, check out a new branch and test the changes. |
…mmunity-survey.md, remove quotes around project.tools json so tools displays as list and not as string representation of list
|
Review ETA: 7/26 EOD |
steven-positive-tran
left a comment
There was a problem hiding this comment.
Branches look good, Code changes seem in line with the issue needed to be fixed and works on my machine.
Thank you for taking this issue for hackforla.
There was a problem hiding this comment.
Hey @yffu - Awesome job on your first issue! Your branches are correct, you linked the original issue, you explained the whats and the whys, and you provided the visuals of the before and after changes.
Bonus points for noticing the issue with current-projects-check.js. It is great and we definitely appreciate that you noticed the problem. If you see something like this in the future, however, PLEASE contact the merge team and let us know what you think might be a problem and how you think the problem can be fixed, so that we can review your solution and make sure it does not cause conflicts elsewhere in the code.
In this case, you are absolutely correct with your changes to current-projects-check.js. Thank you for again for a great job!
Thank you @t-will-gillis for the review and that by @steven-positive-tran. It set my mind at ease about the changes in current-project-checks since it was outside the initial issue description and I wasn't too sure about the different /projects and /projects-check pages... I'll definitely check with the merge team on the hfla-site slack channel about something like this in the future. Thanks again for the review and directions. |
Fixes #4811
What changes did you make?
Why did you make the changes (we will use this info to test)?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals before changes are applied
Visuals after changes are applied
but even with the suggested changes on #4811 tools displays as a string representation of a list, and not a comma separated list. On review of #4861 one of the dependencies for #4811 I discovered that in current-projects-check.js the clause where project.tools is set is still in single quotes. On removing the single quotes tools displays as a comma separated list.