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 todo-progress-bar.json #965

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

8bitgentleman
Copy link
Contributor

adds an option for both horizontal progress bars as well as radial progress bars

image

Just pass in "radial" to the component to try {{[[roam/render]]:((roam-render-todo-progress-bar-cljs)) "radial"}}

Update todo-progress-bar.json

Update todo-progress-bar.json
Copy link

Here’s your link to the diff:

Changed 8bitgentleman/todo-progress-bar bd8b65a → cc28bb7 (PR-shorthand: 8bitgentleman+todo-progress-bar+965)

@baibhavbista
Copy link
Contributor

baibhavbista commented Jan 2, 2025

Hey @8bitgentleman, my review below:

1. Main issue I see is that this change changes the uid of the blocks, both the code and css blocks
image

For example, the codeBlockUid would change from roam-render-todo-progress-cljs to roam-render-todo-progress-bar-cljs

What effect would that have on the old code blocks from before this release?

2. (Optional but would be good to have) I think @tombarys and I ran into a better way of setting-up/tearing-down extensions which have roam render components. Could you take a look?
Some resources:
i. I think this is the relevant roam-depot PR: #869 . And here is the code diff: https://github.com/tombarys/roam-depot-nautilus/compare/08152fd8807663f19b95cd98e88b07b74f4fd38e..e499bb60443abe879d762cbab8c901331d0d0295
ii. Here is (at least part of) the change: tombarys/roam-depot-nautilus#7 . I have a loom video in this link which I think should be enough for you to understand the solution
If you have any questions, feel free to ask away

The new radial view looks nicee btw

@baibhavbista baibhavbista self-requested a review January 2, 2025 07:00
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.

Copy link

github-actions bot commented Jan 2, 2025

Here’s your link to the diff:

Changed 8bitgentleman/todo-progress-bar bd8b65a → 3d45684 (PR-shorthand: 8bitgentleman+todo-progress-bar+965)

@8bitgentleman
Copy link
Contributor Author

@baibhavbista thanks for catching this!! The uid change was definitely accidental. I've also made the load/unload updates mentioned (I believe). I tested and it seemed to work

@baibhavbista
Copy link
Contributor

@8bitgentleman thanks
everything looks good now
And yeah, the load/unload updates looks good too!
will release now

@baibhavbista baibhavbista merged commit 7c8800a into Roam-Research:main Jan 8, 2025
3 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.

2 participants