ChordJS icon indicating copy to clipboard operation
ChordJS copied to clipboard

What about double digit positions and fingers?

Open T-vK opened this issue 8 years ago • 7 comments

The format seems kind of flawed. What do we do if the positions and finger positions are double digit numbers? Do you think we should change it? Or am I missing something?

T-vK avatar Sep 14 '17 13:09 T-vK

This was simply a port of someone's .NET code. They've also done a JS version https://github.com/einaregilsson/chord.js . I was able to get by with things as they are, but please improve as you see fit.

acspike avatar Sep 14 '17 14:09 acspike

I have forked ChordJS and made a couple of changes to it: https://github.com/T-vK/ChordJS/commits/master

Are you interested in a pull request?

What I changed is basically:

  • [change] show the finger positions in the black dots instead of below
  • [fix] correctly re-draw chord diagrams when .replace() is called multiple times
  • [fix] prevent crashing when a position/fingering with more than 6 characters is used
  • [new] I called the variable ChordJS instead of chords (but chords is also kept for backwards compatibility)
  • [new] I split ReplaceChordElements into two functions, the new one is called GenerateChordHtml
  • [new] GenerateChordHtml is exposed as .generate() and can be used to generate a canvas without having to use markup html.
  • [new] ReplaceChordElements has an optional parameter now to specify an element on which all child chord elements should be replaced.

It looks different now: https://github.com/T-vK/Chord-Draw

T-vK avatar Sep 14 '17 22:09 T-vK

Lovely. Would it be possible to keep the numbers below the strings as an option for small chord charts? And it looks like the numbers could get nudged down a little lower in the dot. I would happily accept a pull request.

acspike avatar Sep 14 '17 23:09 acspike

Great.
I agree the numbers should be positioned a little lower. But if I just move it down by 3px it wouldn't scale very well. Any ideas on how to center them in a more elegant way?

I think we should add another option so that the user can always decide which layout he wants, instead of making it dependent on the size. I should also add an option to let the user specify the string names, in case a different tuning is used.

Edit: I added an option to specify which layout to use and another option set the string names.

T-vK avatar Sep 15 '17 07:09 T-vK

Need better browser support for font metrics. https://developer.mozilla.org/en-US/docs/Web/API/TextMetrics Perhaps there's some common work around out there.

acspike avatar Sep 15 '17 11:09 acspike

Comment about the original heading: I personally doubt that anybody needs a two digit finger number :)) But... to use the position format "8-8-10-11-12-8" (for example) you have to modify the "if" statement in the "GenerateChordHtml" function (line 471) to allow position strings with more than 6 characters. After that barre-chords on the higher frets work fine...

ghost avatar Jun 09 '20 12:06 ghost

Interesting, now that you say that, I totally missed line 201. I always thought the only supported format was a 6 characters string. If this actually worked for you, I think you should make a pull request.

Reading through my old posts I see that I must have tried to fix a bug that caused crashes ([fix] prevent crashing when a position/fingering with more than 6 characters is used).
I would suspect those crashes only happened when you would pass a string longer than 6 digits which doesn't contains any dashes. For example I think 1122334 would cause a crash. So the way to go imo is to change

if (positions.length != 6 || fingering.length != 6) {

to something like

if ((positions.length != 6 && !positions.includes('-')) || fingering.length != 6) {

I personally doubt that anybody needs a two digit finger number :))

You would be surprised.

T-vK avatar Jun 09 '20 14:06 T-vK