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 hide border option #52

Merged
merged 2 commits into from
Feb 18, 2021

Conversation

DenverCoder1
Copy link
Contributor

All Submissions

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions

  1. Does your submission pass tests?
  2. Have you lint your code locally prior to submission?

Changes to Core Features

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully ran tests with your changes locally?

Feature changes

  • Added a hide_border query parameter which will set the stroke of the outer rectangle to #0000 (transparent) when set to true.
  • Using both the theme parameter and other options will allow the user to use a theme as their "base" and overwrite specific properties. This allows the user, for example, to pick a theme and decide to hide the border or change the background while keeping the other options from the theme.

Screenshots

http://localhost:5100/graph?username=DenverCoder1&theme=redical

image

http://localhost:5100/graph?username=DenverCoder1&theme=redical&hideBorder=true

image

@DenverCoder1
Copy link
Contributor Author

Closes #51.

Will add another commit to update the readme.

@Ashutosh00710 Ashutosh00710 self-requested a review February 18, 2021 15:29
Copy link
Owner

@Ashutosh00710 Ashutosh00710 left a comment

Choose a reason for hiding this comment

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

Awesome changes @DenverCoder1. Thanks for contributing in this project. Merging this to main.

@Ashutosh00710 Ashutosh00710 merged commit abf6625 into Ashutosh00710:main Feb 18, 2021
@Kshitij978
Copy link
Collaborator

Closes #51.

Will add another commit to update the readme.

Okay, go for it. 👍🏼

@DenverCoder1
Copy link
Contributor Author

@Kshitij978 It seems this PR is closed, should I open another PR to add it?

@Ashutosh00710
Copy link
Owner

Yes, @DenverCoder1 🙂

@DenverCoder1 DenverCoder1 mentioned this pull request Feb 18, 2021
7 tasks
@DenverCoder1
Copy link
Contributor Author

Yes, @DenverCoder1 🙂

Done. #53

Thanks for allowing me to contribute to this awesome project! 🎉

@Ashutosh00710
Copy link
Owner

You're welcome 🙂 @DenverCoder1

@Kshitij978
Copy link
Collaborator

@DenverCoder1 Changes have been deployed. This feature is now available in production. 🎉
Thank you for adding value to this project. 🙌🏼

@DenverCoder1
Copy link
Contributor Author

@DenverCoder1 Changes have been deployed. This feature is now available in production. 🎉

Awesome! 🎉

I tried it out and it seems hideBorder works but hide_border doesn't. I changed the name of the parameter when adding the docs in #53 and it seems that the newest change may have not been deployed.

@Ashutosh00710
Copy link
Owner

Deployed Again. Fixed this.
That was by mistake.
Thanks for reminding me.

Now check hide_border parameter.

@DenverCoder1
Copy link
Contributor Author

@Ashutosh00710 It works! 🎉

@Ashutosh00710
Copy link
Owner

@DenverCoder1, I visited your profile and it was really awesome. Your color choice for the activity graph was so good.

I was thinking about adding your color combination as one of our themes.

  • Do you agree to this?
  • If yes, can you name the theme and make a PR for issue Add more themes #25 or you want us to add this?
  • If no, no problem that's completely fine.

@DenverCoder1
Copy link
Contributor Author

@Ashutosh00710 Sure!

@DenverCoder1 DenverCoder1 mentioned this pull request Feb 19, 2021
4 tasks
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.

3 participants