Skip to content

Allow users to specify abcjsParams #11162

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

Merged
merged 2 commits into from
Apr 27, 2024
Merged

Conversation

Clegs
Copy link
Contributor

@Clegs Clegs commented Apr 26, 2024

Currently the parameters for abcjs are hard-coded to:

{
  "responsive": "resize"
}

This prohibits the user from using other options, such as tablature.

For example, if I want to render this violin tab in the abcjs documentation, I cannot do it because there is no way to set the abcjsParams to:

{
  "tablature": [
    { "instrument": "violin" }
  ]
}

This PR allows users to manually specify abcjsParams to use by prefixing the content with:

%%params {tablature: [{instrument: "violin"}]}

Using this, the example now renders correctly:

%%params {tablature: [{instrument: "violin"}]}
X:1
T: Cooley's
M: 4/4
L: 1/8
R: reel
K: G
|:D2|EB{c}BA B2 EB|~B2 AB dBAG|FDAD BDAD|FDAD dAFD|

Produces:
image

Please let me know if there is another approach to get the abcjsParams passed into the render function that you would prefer.

  • Please commit to the dev branch
  • For contributing new features, please supplement and improve the corresponding user guide documents
  • For bug fixes, please describe the problem and solution via code comments
  • For text improvements (such as typos and wording adjustments), please submit directly

EDIT: Updated the code to use the new format

Sorry, something went wrong.

@88250 88250 requested a review from Vanessa219 April 27, 2024 00:35
@@ -3,6 +3,30 @@ import {Constants} from "../../constants";
import {genIconHTML} from "./util";
import {hasClosestByClassName} from "../util/hasClosest";

const ABCJS_PARAMS_KEY = "%%abcjsParams:";
Copy link
Member

Choose a reason for hiding this comment

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

Would it be better to use %%params {responsive: "resize"}?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds good to me. I'll update it to use that as well as looseJsonParse that you mentioned below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the code. Let me know what you think.

@Vanessa219 Vanessa219 merged commit d9dedb5 into siyuan-note:dev Apr 27, 2024
1 check passed
Vanessa219 added a commit that referenced this pull request Apr 27, 2024
@Vanessa219 Vanessa219 added this to the 3.0.12 milestone Apr 27, 2024
Vanessa219 added a commit that referenced this pull request Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants