Graphite icon indicating copy to clipboard operation
Graphite copied to clipboard

Create a Radio button for sample by count option

Open Moharum1 opened this issue 6 months ago • 4 comments

Closes #2720

Moharum1 avatar Jun 17 '25 13:06 Moharum1

@Keavon FIxied the points you mentioned above

Moharum1 avatar Jun 18 '25 09:06 Moharum1

!build

Keavon avatar Jun 18 '25 09:06 Keavon

📦 Build Complete for c3667ca33b775c41c9c38c05c05c57f745c2df38
https://03867918.graphite.pages.dev

github-actions[bot] avatar Jun 18 '25 10:06 github-actions[bot]

Please ensure this works with the adaptive spacing parameter.

Keavon avatar Jun 18 '25 10:06 Keavon

@Keavon but isn't it already adaptive without the need of the adpative spacing option, i mean the code distruibute the lenght equally on the number of points specified, so what should an adaptive spacing do ? i will do some search but further explination would be nice

Moharum1 avatar Jun 19 '25 13:06 Moharum1

Please test how the node works with adaptive spacing on and off. You'll see how it isn't incompatible with your new mode.

Keavon avatar Jun 19 '25 20:06 Keavon

I have force pushed this branch to move it to a new base. Ensure you have the branch checked out. Then please run git stash on any local changes (and ask me before following these steps if you have any un-pushed commits on your local machine). Then fetch this branch and git reset --hard origin/Radio-button-for-sample-by-count-option. And finally git stash pop your stashed local changes, if you stashed them.

Keavon avatar Jun 20 '25 07:06 Keavon

I've marked this as a draft while waiting on the requested change. Please mark this as "ready for review" once completed. Thank you!

Keavon avatar Jun 23 '25 11:06 Keavon

@Keavon Is there any problem with the re-written version ?

Moharum1 avatar Jun 25 '25 08:06 Moharum1

I'm currently finishing up my work on getting this ready to merge.

Also I realized that you were right about "count" not needing "adaptive spacing", I'd misunderstood that when I was testing it. Sorry about that confusion.

Should be merging shortly.

Keavon avatar Jun 25 '25 08:06 Keavon

Please remember to leave a comment in #2720 with this PR number.

Keavon avatar Jun 26 '25 07:06 Keavon