Skip to content

Added a reference to GraphicalNote from Note #659

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

Closed
wants to merge 6 commits into from
Closed

Added a reference to GraphicalNote from Note #659

wants to merge 6 commits into from

Conversation

praisethemoon
Copy link
Collaborator

Hello!

I believe that this tiny change would really prove usefull as it is strongly related to #549 in the sense that it would be easier to get the graphical note object from, for example cursor usingosmd.cursor.iterator.currentVoiceEntries[0].notes[0].graphicalNote.getSVGGElement().id which is more convinient than fetching it from osmd.GraphicSheet.MeasureList[0][0].staffEntries[0].graphicalVoiceEntries[0].notes[0]. If there is something that already exists and solves this problem, please let me know!

For me this is the best way to hack into the low-level SVG generated objects to change their style, and I do hope you find it useful!

Cheers!

@sschmidTU
Copy link
Contributor

Hey, i basically answered this in a new issue in #660.
The thing is, we wanted to avoid creating references to graphical classes in sheet model classes, to modularize OSMD and make it possible to use the sheet model without the graphics classes.
It should be almost as easy with the solution in #660 though. We can also add a static method to Cursor or GraphicalNote that gets you the GraphicalNote from a Note via the new dictionary.

sschmidTU pushed a commit that referenced this pull request Jan 22, 2020
@Jack-Works
Copy link

How does this PR going? 👀

@sschmidTU
Copy link
Contributor

sschmidTU commented Apr 2, 2020

@Jack-Works As i said, we do not want a direct reference to graphical classes in our sheet model classes, but hopefully #660 will get resolved and then you will have a way to find a GraphicalNote from a Note that is almost as easy as a reference in Note.

@ice6
Copy link

ice6 commented May 16, 2020

@praisethemoon this is a great feature, when you design an interactive sheet music app.

@sschmidTU this library is a bridge layer between musicxml and vexflow, it is inevitable that you have to interact with the ui layer from model layer.

Maybe we can add a GraphicalNote property to Note.ts with datatype any, if you don't want to make a circular dependency.

With @praisethemoon 's change, we can access HTML element from the note data happily like this.

osmd.cursor.iterator.currentVoiceEntries[0].notes[0].graphicalNote.vfnote[0].attrs.el

If we dig the code deeper, I think there is no reason for AJAX.ts existing in this library. 😄

@ice6
Copy link

ice6 commented May 16, 2020

@sschmidTU

After rethinking, maybe we just need to add a currentGraphicalNotes attribute to osmd.cursor.iterator.

This can solve many ui interaction problems.

@matt-uib
Copy link
Collaborator

@ice6 thanks for your comments here also.
Our idea is to keep the general objectmodel built up from musicxml and also the cursor clean from the final graphical implementations. An Idea is for example: a musicxml audio player without even rendering any notes.
Our solution would be the dict described in #660 . Would you see any problems with this approach?

@ice6
Copy link

ice6 commented May 16, 2020

After more digging, I found that osmd.cursor.iterator.currentMeasure.verticalMeasureList[0].staffEntries[0].graphicalVoiceEntries[0].notes[0].vfnote[0].attrs.el, this already can solve many ui interaction requirements.

@matt-uib it is ok to use dictionary to solve the problem. and the final api is clean. If it is already implemented, I would not discuss here. :P

ice6 added a commit to ice6/opensheetmusicdisplay that referenced this pull request May 18, 2020
@sschmidTU
Copy link
Contributor

sschmidTU commented May 18, 2020

If we dig the code deeper, I think there is no reason for AJAX.ts existing in this library. 😄

@ice6 AJAX is used in OpenSheetMusicDisplay.ts:load() to retrieve a MusicXML file asynchronously from a website.

@ice6
Copy link

ice6 commented May 21, 2020

If we dig the code deeper, I think there is no reason for AJAX.ts existing in this library. 😄

@ice6 AJAX is used in OpenSheetMusicDisplay.ts:load() to retrieve a MusicXML file asynchronously from a website.

@sschmidTU sure, I know. just from the architecture perspective.

below is the code I use to retrieve file by url, it provides more flexibility some time.

let res = await fetch(url)
let content = await res.text()

sschmidTU pushed a commit that referenced this pull request Jan 25, 2021
…es) (#660, #659)

fix #660
close #660

replaces a potential reference to the GraphicalNote in Note,
because we want to keep the data sheet model separate from the graphical model,
so we don't want a reference to GraphicalNote in Note.
@sschmidTU
Copy link
Contributor

There is now the static GraphicalNote.FromNote(note: Note, rules: EngravingRules),
so you can easily get the GraphicalNote like this:
const gNote = GraphicalNote.FromNote(note, osmd.rules);

See #660.

@sschmidTU sschmidTU closed this Jan 25, 2021
@sschmidTU
Copy link
Contributor

sschmidTU commented Jan 25, 2021

There is an even easier method now:
const gNote = osmd.rules.GNote(note) (in typescript: osmd.EngravingRules)
will give you the GraphicalNote from a Note reference.

sschmidTU pushed a commit that referenced this pull request Jan 25, 2021
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.

None yet

5 participants