Skip to content

Conversation

@manumeter
Copy link

PR for #125 (as requested).

@estellecomment
Copy link

Great, thanks ! ❤️

The display looks a bit different on my machine than for you... For "4/4 with triplets", the X are now very close and stepping on top of each other. I'm not sure if I'm clicking in the right place or not. A little bit more space would be nice.
image

When I switch to "24" I get this :D (looks like veeeerrrryyy faasssstt playing)
image

@manumeter
Copy link
Author

Yes, this is actually intentional because I tried to improve the situation in upstream where, if you have some triplets, doubles, shoutings or so, the whole table column shifts and what should be evenly spaced by sound is not anymore visually, so I switched to a fixed with per beat. And for patterns with a high amount of subdivisions, this requires (visually) overlapping strokes, but this should actually never happen in reality, because patterns with 12 or even 24 subdivisions don't need this because they are so fast but because they combine different tempos (like doubles and triplets). So you just need to leave the correct amount of empty spots between your strokes (depending on the tempo). Or do you actually have a use-case for that many strokes per beat?

By increasing the width per beat, the table gets very wide for something like 24 intersections with no real need, which decreases readability in my opinion. (Like in the screenshot here: #125 (comment))

@manumeter
Copy link
Author

But, what would be a nice feature idea: When changing the subdivisions, the pattern could be streched/shrinked to the new time signature. Of course shrinking would not be possible without loss.

@estellecomment
Copy link

I think the misunderstanding is this : with 24 subdivisions, I'm indeed able to write rhythms in code, but not able to compose them by clicking in the interface, because the subdivisions are too narrow.

Example : I have written a rhythm in the code for the repique. In the interface, I try to make the snare play the same rhythm as the repique. This happens :

Screen.Recording.2025-11-27.at.19.57.33.mov

In the interface, with the fixed width and the very tiny subdivisions, I cannot click precisely enough to place my strokes, I don't really know if I'm clicking at the right spot. It's also difficult to remove a stroke, because it's hard to click on the stroke itself, I often click on an empty space next to it instead.

Up to 12 subdivisions ("4/4 with triplets") it's still doable 👍 (though also a bit tricky)

I do agree that before this PR, like in the screenshot, the width is too much, it makes it hard to read.
Could we use something in between?
Or use a fixed width if time <= 8, and a width proportional to time if time > 8 (for example) ?

UX-wise, I think the best way would be to allow to change from binary to ternary by hand for each beat, and thus display only the necessary number of subdivisions (either 8 or 6, rather than 24), which would make it easy to use and understand. But I don't have an easy way to do this :)

@estellecomment
Copy link

Actually, I think we should move this width debate in another PR. Because the display of triplets is really the point here, and will be helpful regardless of width. What do you think ?

}

&.is-triplet {
color: #0000ff;

Choose a reason for hiding this comment

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

This does not work in the Compose tab, because strokes are < a > links. This rule is overriding the color change :
Screenshot 2025-11-27 at 20 13 53

Also, < a > links are blue by default, so maybe green would be a good color for triplets ? (or something else)

This would work for example :

                &.is-triplet {
					color: var(--bs-green);

					a {
						color: var(--bs-green);
					}
                }


if(hasNote(idx)) {
for (let step of [2, 4, 8]) {
// CASE 1: idx is on the right note of a triplet

Choose a reason for hiding this comment

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

would you consider giving graphical examples, things like : X X, so that it's easier to understand ? I'm struggling. :)

Copy link
Author

@manumeter manumeter Nov 28, 2025

Choose a reason for hiding this comment

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

Okay, I try to visually explain the different cases:

          idx
           |
           v
 CASE1:  X.X
       X...X
   X.......X
 CASE2:    X.X
           X...X
           X.......X
 CASE3:   X.X
        X...X
         X.......X
 CASE4: |........X...|X...X..

BTW this function could be made more efficient for sure (it's slowing down everything for large patterns, but I don't know how to elegantly solve this without the recursion I used).

@manumeter
Copy link
Author

Hmm, I see your issue. (Never was one for me because not many people write rhythms in our player and if I do it, I use the keyboard, especially for more complicated stuff.)

Maybe we should separate the use-case of listening and the use-case of composing patterns. In the listen UI, I still think evenly spaced bars make the most sense, independent of the number of subdivisions. But we could increase the width in compose mode to make it more user friendly. What do you think?

And regarding colors: I thought the coloring of triplets in the compose mode is not important, only in the listening UI. But I can change that to also color links if you want. (And I chose blue because it's already an existing convention with us to have the triplets in blue. But we can change that upstream if needed and I will patch it back to blue for our instance.)

Regarding splitting the PR: Hmm, you're right, strictly speaking, it's a different issue. But since the triplet thing also requires a lot of CSS changes in the same area, I'd rather keep it together to avoid having two conflicting PRs.

@estellecomment
Copy link

Thanks for the answer ! It's nice to have a thoughtful and peaceful conversation :)

  • colors : You're right that following the existing color code is best. I'd also go for using the same colors for Compose and Listen (to avoid unnecessary complexity for the user). For me, when composing, the color change for triplets is also useful.
    So, simplest solution I can see : make strokes black by default in Compose mode (right now they are blue), so that it's the same as Listen mode. Then the color change from triplets will be the same as well.

  • spacing : I agree that for Listen mode, when the bars are too long it's annoying, you have to scroll sideways. So ok, differentiating Compose and Listen can work, good idea!
    A technical note : using px as a width unit means that if we change font size, the width will be all wrong. I'm not sure what happens either for visually impaired users when they change the default font size in the browser or in the OS... I would suggest using em (or ex like the pre-existing code) to avoid these problems.

  • subdivisions : I love that you displayed only 4 subdivisions per beat (with border-right), rather than display it for all strokes like it was before (except for triplets, 3 instead of 4, of course :) ). It's much clearer for me, both in Listen and in Compose modes.
    For the implementation, I would suggest a simplification : currently, the code sets a visual border-right for all strokes, and then in your PR you remove the ones you don't need. It could be simpler to not set border-right for strokes initially, and then only set it where necessary (and differentiate the triplet vs. non-triplet case). For humans reading the code it would be easier to understand.

  • computing is-triplet more quickly : thinking about it.

I think a lot of the difficulty in the algorithm is because triplet-or-not (a.k.a ternary or binary) is actually a property of the whole beat, not the individual stroke. You can't have a beat with half-binary and half-ternary, right? It's either binary or ternary.

Only some strokes are acceptable for a binary rhythm :
For a beat with time = 12, the X are the allowed binary strokes : ||X..|X..|X..|X..
For a beat with time = 24, the X are the allowed binary strokes : ||X..X..|X..X..|X..X..|X..X..
The others (represented with dots) are not possible in a binary rhythm, the are not a division by 2 of something.

So if the user uses any of non-binary strokes, then the whole beat is ternary.
So the algorithm becomes (in pseudo code) :

const isTernary = (beat) => {
    for each strokeIdx in the beat {
        if(strokeIdx%3 !== 0 && hasBeat(strokeIdx))
            return true` (the beat is ternary)
    }
 }

It would be easier to add a beat-level HTML element, parent of the strokes, to which we could add the is-Triplet CSS class when needed. Then the CSS would be easy. (it does mess up the nice HTML table... not sure exactly what's the best way to do it but I think it's worth it)

  • UX thought : Another possibility is to let the user chose whether they want to display 3 or 4 subdivisions, with a clickable option. If time%3 == 0 then display the option to let them chose, for each beat. It's additional work for the user and it adds clutter to the UI, but I maybe it might be easier to understand for the user ? Or not. (but it avoids the tricky algorithm 😂 )

@estellecomment
Copy link

Here's a commit (on my own branch) implementing the isTernary function that I'm describing.
manumeter#1

For now I'm not adding the beat-level HTML element, it's trickier than I thought (HTML table and v-for are limiting), so it's still wasteful : the computation is run for each stroke (instead of each beat).

@manumeter
Copy link
Author

Wow, your solution is much simpler & more elegant. Thanks a lot! I've tested it on our rhythms and it works perfect.
Only difference I noticed: I've considered the first hit of the next beat being part of the triplets:

Before:
before

After:
after

But I can also live perfectly fine with this solution because I think it's also elegant to define it on beat level rather than on stroke level.
And I did not test your solution for upbeats (but we're not having upbeat-triplets in our repertoire so far, so it's not critical for us).

@manumeter
Copy link
Author

Regarding colors/spacing/subdivisons and simplification of the CSS part: I'll try to make a proposal when I have time (maybe not this week) - or if you want to do it, let me know.

(I also want to try out how user-friendly it would be to have visible sub-subdevisions in compose-mode (like the 4 subdivisons as always and everything between in an even lighter gray and with more space.)

@estellecomment
Copy link

I'm glad that the algorithm is helpful ! I would vote for leaving the stroke of the next beat in black, yes, mostly because it would add complication to an otherwise simple plan... I can see how it's a little less user-friendly. 🤷‍♀️

Subsubdivisions sounds useful for time 24, yes, I agree !

Here's a task list of the things that I can think of :

  • 1. more spacing for Compose (leave Listen as it is)
  • 2. test the triplet algorithm for upbeats (I didn't test it either), fix if needed. Unit tests ???
  • 3. change colors in Compose to match Listen
  • 4. simplify CSS for subdivisions : set classes only once where needed, rather than setting and removing
  • 5. subsubdivisions (maybe for a different PR ? this PR has sizeable things already, and I think it can be an independent task)

I'm going to start on the first two. I'll update you as I go so that we can share out tasks depending on available time.

@estellecomment
Copy link

estellecomment commented Dec 3, 2025

I got sidetracked by a new problem concerning width : some strokes, like "rim", have names that are longer than "X", so that they don't fit within the current fixed width. "hd" and "sil" are also among the longest. And then the shouts...

Examples :

Screenshot 2025-12-02 at 16 41 31 Screenshot 2025-12-02 at 16 48 46 Screenshot 2025-12-02 at 16 39 08

I would very much like to merge this PR into the original ror-player, so I'm not comfortable with breaking the layout of existing ror rhythms... 🤔

ror-player has flexible widths to deal with shouts : they stretch the column that they belong to, if needed. This is ok as long as all columns are visibly separated, because you can still understand the rhythm. For time 12 and 24, with many tiny invisible subdivisions, it's not really an option, the rhythm becomes really impossible to read, you can't guess where the invisible columns are. So I think fixed-width, as you're already doing, is the way to go.

Since I'd like to change the units from "px" to "em" anyway, I'm going through the different widths to see what would need changing to support "rim".
For smaller values of "time", I think we don't need to squeeze so much, so leaving a bit of extra space seems fine to me.
For 8, 12, and 24, we could have two sets of values : "with rim strokes" vs. "with only X strokes".
(We could also have two sets of values for all times, so you can keep the same layout as before this PR, if you prefer. No strong feelings on this one really.)

@manumeter
Copy link
Author

manumeter commented Dec 3, 2025

Oh, I didn't have in mind the "rim", "sil", "hd" strokes. In my personal opinion, the most user friendly way would be to replace them by "r", "s", "h" (or so) and have a mouse-over text that explains the stroke types (as it is already implemented). That's because I find the patterns much better to read and understand on a narrower table. But of course we have to adapt to what the RoRies and core dev(s) prefer. And if it is to stick with these stroke words, then I guess the table needs to become quite a lot wider (depending on the subdivisions and which use-cases should be supported, like double rim-strokes for example) or non-static in width (which I personally dislike).

But regarding the shoutings: I think this is actually much better like this. They've been the second important reasons for me to switch to fixed with, otherwise the table gets distorted (sometimes quite badly). What I've tried to improve regarding UX (but wasn't able to achieve in CSS) was to remove/hide the lines behind the (overlapping) shouting text.

Here are two examples with this patch (and my proposed solution):

Tune before/after:
Screenshot from 2025-12-03 19-58-50
Screenshot from 2025-12-03 19-58-54

Break before/after:
Screenshot from 2025-12-03 19-59-14
Screenshot from 2025-12-03 19-59-19

But in the end, this is all a matter of taste/preference, and I have no clue about the RoR communities preferences, I just adapted it to fit our needs better.

@estellecomment
Copy link

Thanks for the examples, that makes a lot of sense. There is indeed a matter of preference on long vs. short notations.

Since it looks like many matters are to be considered on many subjects, I think I'm going to go back to a previous idea and produce a PR with just the triplets stuff, and no change in width anywhere. Because it would be a small scope and easier to discuss and merge into ror-player (I haven't given up on merging yet :) ).

It's really nice to have a discussion about all this, I'm learning and finding new ideas. I think the different pieces will eventually find their way in, good stuff :)

@estellecomment
Copy link

Here goes : PR with just the triplets stuff, no changes in width. #136

…rs; shouting-text overlay; border-css cleanup
@manumeter
Copy link
Author

Okay great that you offer them a minimal version as well.

I've worked a bit on this one anyways.

  • Upbeats break the triplet calculation, so this needs some fixing.
  • Changing to em/ex for the table widths didn't really work for me, so I stick with px for now.

Other than that, I think I managed to solve the points we discussed here so far (maybe some things could be done a bit nicer, but at least we have a proof of concept for now).

@manumeter
Copy link
Author

Okay with this last commit, triplets are still not working in upbeats, but upbeats don't screw up triplet-calculation in the regular beats anymore.

@estellecomment
Copy link

I fixed upbeats and used a less wasteful implementation in the "simple version" PR : #136
I can port it back into this one if you like ?

@manumeter
Copy link
Author

That would actually be very nice, I'm currently using this PR for our fork.

@estellecomment
Copy link

Here goes : manumeter#2

More efficient triplet computation, and pink color
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