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

Added card_width argument to gist card #3405

Closed
wants to merge 3 commits into from
Closed

Added card_width argument to gist card #3405

wants to merge 3 commits into from

Conversation

yaten2302
Copy link

fixes #3370

This PR adds a new argument (card_width) to the gist-card, which allows the user to adjust the width of the card.

@vercel
Copy link

vercel bot commented Oct 22, 2023

@yaten2302 is attempting to deploy a commit to the github readme stats Team on Vercel.

A member of the Team first needs to authorize it.

@yaten2302
Copy link
Author

Hi @qwerty541, @rickstaa, could you please review and merge my PR under hacktoberfest.

src/cards/gist-card.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@rickstaa rickstaa left a comment

Choose a reason for hiding this comment

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

@yaten2302 thanks for your pull request! Your pull request looks good 🚀. There however are some things that need to be resolved:

  • The new parameter has to be documented under the Gist card exclusive options.
  • Tests need to be added.
  • Logic needs to be added so that the width gets clamped at a minimum width.

It would be great if you could fix these issues so that we can merge your PR in the main branch. For now I attached the hacktoberfest-accepted label since your pull request adheres to all Hacktoberfest requirements.

@yaten2302
Copy link
Author

Thanks for the review @rickstaa, I'll make the changes ASAP👍

Added `MIN_CARD_WIDTH` to prevent the card from becoming too small.
@yaten2302
Copy link
Author

Hi @rickstaa, made the changes, please review 👍

@rickstaa
Copy link
Collaborator

rickstaa commented Nov 6, 2023

Hi @rickstaa, made the changes, please review 👍

Hey, @yaten2302 are you sure you pushed the changes to the remote? I can't find the changes on GitHub 🤔.

@yaten2302
Copy link
Author

image
@rickstaa, are the changes not showing up on your side? Should I create a new PR to fix this?

@rickstaa
Copy link
Collaborator

rickstaa commented Nov 9, 2023

@yaten2302 thanks for your pull request! Your pull request looks good 🚀. There however are some things that need to be resolved:

  • The new parameter has to be documented under the Gist card exclusive options.
  • Tests need to be added.
  • Logic needs to be added so that the width gets clamped at a minimum width.

It would be great if you could fix these issues so that we can merge your PR in the main branch. For now I attached the hacktoberfest-accepted label since your pull request adheres to all Hacktoberfest requirements.

@yaten2302 I also requested these changes (I.e. documentation and tests). Sorry for being unclear 😅. Please let me know if you have any questions.

@yaten2302
Copy link
Author

@rickstaa, sorry, I forgot to make the changes in the tests and in documentation. And also, there I'm facing some issues in committing and pushing the changes, I will have to close this PR and create a new one. Sorry for inconvenience 🙏

@yaten2302 yaten2302 closed this Nov 10, 2023
@rickstaa
Copy link
Collaborator

@rickstaa, sorry, I forgot to make the changes in the tests and in documentation. And also, there I'm facing some issues in committing and pushing the changes, I will have to close this PR and create a new one. Sorry for inconvenience 🙏

No problem feel free to let me know when you created the new pull request 👍🏻.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add gist card card_width argument
2 participants