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

New extension: Contribution Graph (Github-like) #887

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Dagulf795
Copy link

No description provided.

Copy link

Here’s your link to the diff:

Added Dagulf795/contribution-graph 09c8eaf (PR-shorthand: Dagulf795+contribution-graph+887)

Copy link
Contributor

@baibhavbista baibhavbista left a comment

Choose a reason for hiding this comment

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

@Dagulf795 looks like a really fun extension. I left some comments requesting changes below:

  • Suggestions/requested changes

    1. Would prefer a setting to make it available via a command (in cmd palette) instead of something that always takes up space in my navigation bar
    2. add gif or screenshots in README.md
    3. package.json cleanup
      • can you remove the commands you're not using from package.json
        • I think these exist because this repo started from a clone of phonetonote/similar-pages
      • can you remove dependencies you're not using?
        • mathjs
        • maybe others too?
    4. when multiple years, show the year too
      • see example of a graph which has gone on multiple years
    5. Why do you have server side rendering turned on?
      • it seems most of the code complexity comes from the fact that you're getting svg from the module (I assume this is because of the ssr setting?). Would it not have been easier if using client side rendering?

@baibhavbista baibhavbista changed the title Add Contribution Graph extension metadata New extension: Contribution Graph (Github-like) Jun 13, 2024
@Dagulf795
Copy link
Author

Hi @baibhavbista - changes now made. See here

Thanks!

Copy link
Contributor

@baibhavbista baibhavbista left a comment

Choose a reason for hiding this comment

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

@Dagulf795 you need to update the source_commit to the version of the repo you want to build. Please check my comment below with clairification

"tags": ["activity", "contribution graph", "contribution", "GitHub"],
"source_url": "https://github.com/Dagulf795/contribution-graph",
"source_repo": "https://github.com/Dagulf795/contribution-graph.git",
"source_commit": "09c8eaf5e39d8c3dee789b6607ce9fff40f6f0e2"
Copy link
Contributor

Choose a reason for hiding this comment

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

@Dagulf795 it looks like you solved most of the issues I asked, thank you

Just one issue, after making an update to the source code for the extension, you need to update the source_commit here in the JSON

To make sure the extension matches how it works locally, I also recommend checking your extension using the PR-shorthand (Mentioning this because testing via this would have caught the issue above)

image

You'd use that via the "load extension from URL" option in Settings > Roam Depot > developer Extensions (screenshot below)
image

Added corrected source_commit
Copy link

github-actions bot commented Jul 1, 2024

Here’s your link to the diff:

Added Dagulf795/contribution-graph 0f0274c (PR-shorthand: Dagulf795+contribution-graph+887)

@baibhavbista
Copy link
Contributor

Hey @Dagulf795
Super sorry for the late reply and re-review!! I was a bit busy with other work and this ended up slipping through.
I found a few issues:

  1. the default value for the "Show Contribution Graph Button" is off, but by default it is shown

    • you seem to have init code for this but it does not run because the check for when this runs is incorrect
      • required fix looks like
        • image
  2. sometimes I was able to get it to this state (can't really find a reproduction for this bug, it seemed to happen more regularly when the contribution button was the first thing I pressed)

    • image

I do know that this PR has been almost ready for such a long time, so if you'd prefer for me to release it as it is, and you fix them later, I am totally fine with that too. Please let me know

And again, sorry for getting back so late!

@baibhavbista
Copy link
Contributor

@Dagulf795 checking in about this again. Do you want me to release with the issues (& later fix it) or fix the issues before release?

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.

2 participants