Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update-contributors-section #64

Merged
merged 7 commits into from
Feb 24, 2024

Conversation

mmcbride2929
Copy link
Collaborator

@mmcbride2929 mmcbride2929 commented Aug 30, 2023

TL/DR

This PR updates members section, and links to the contributors graph page.`

Overview of Change

Members.jsx

  • updates members img
  • updates members array with name & id of each member
  • wrapped contributors img with an a tag, to link to contributors graph

index.css

  • set members container width to 1000px to match admin section
  • set members contributors-name-container to align names with corresponding photo'

Untitled

@netlify
Copy link

netlify bot commented Aug 30, 2023

Deploy Preview for dev-fcccolumbus ready!

Name Link
🔨 Latest commit 5a4c829
🔍 Latest deploy log https://app.netlify.com/sites/dev-fcccolumbus/deploys/65da425bb9644d0008931f0d
😎 Deploy Preview https://deploy-preview-64--dev-fcccolumbus.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@readysetagile
Copy link
Collaborator

@mmcbride2929 now that the lint issues are fixed you can re-run your checks in this PR by pushing an empty commit:

git checkout <this branch>
git commit -m "retrigger checks" --allow-empty
git push ...

@readysetagile readysetagile linked an issue Sep 1, 2023 that may be closed by this pull request
@readysetagile readysetagile linked an issue Sep 1, 2023 that may be closed by this pull request
@readysetagile readysetagile linked an issue Sep 1, 2023 that may be closed by this pull request
Copy link
Collaborator

@readysetagile readysetagile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually - nix my last comment

the contributors section pictures should reflect what is in the Contributors section of the repo. So you should be pictured there and you are not. See the requirement...

GIVEN I am a FreeCodeCamp user
WHEN I view the "Meet the Contributors" section
THEN I will see the image of contributors generated by the [contrib.rocks](https://contrib.rocks/image?repo=FCCColumbus/cbus-web) website

@jduffey
Copy link
Collaborator

jduffey commented Sep 2, 2023

npm test is failing for me at 704bc83. Need to update snapshot for the Members component.

src/components/Members.jsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@readysetagile readysetagile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to remove the members structure and all the supporting code. The names below each person's icon is not part of the requirement. The reason for this is because the image at contrib.rocks is dynamically created every time a new member has code merged into our repo. We do not want to change the contributors section every time a new member merges code. Clicking on the contributors image will give our members an "identity" by navigating the user to the gihub activity graph.

The snapshot test for the component will also need to be changed, but it should be much much simpler...

src/components/Members.jsx Outdated Show resolved Hide resolved
src/components/Members.jsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@jduffey jduffey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, nice work! :shipit:

@StevenH34
Copy link
Collaborator

What is the status of this PR?

@readysetagile
Copy link
Collaborator

No merge was performed before #75 was merged. @jsolly identified the weirdness of the commits and I misdiagnosed it during the hackathon. looked at both PRs, and I am thinking that when I approve #75 that the changes for this PR will still be OK. I'll re-run the checks again for completeness.

@readysetagile readysetagile merged commit a04ffdd into FCCColumbus:dev Feb 24, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Meet the Contributors section
5 participants