-
Notifications
You must be signed in to change notification settings - Fork 36
Updated documentation for the changes to the EJS_Buttons variable #55
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
base: main
Are you sure you want to change the base?
Conversation
@@ -1,5 +1,11 @@ | |||
## Main Options | |||
|
|||
::alert{type="info"} | |||
::list{type="info"} | |||
- **Note**: Some projects have been using internal API's not documented here. As these internal API's are undocumented, they are not guarrenteed to function the same between EmulatorJS versions. We *highly* recommend not using internal API's. If they are something your project relies upon, consider raising a Pull Request or Feature Request to have the API exposed public and documented here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought it might be prudent to put this alert on this page. Devs accessing internal functions and API's is never a good idea as we can get hamstrung by trying to maintain compatibility with internal functions. We should only support ones we've publicly documented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I like the idea - we have no internal APIs documented at the moment, which would make gaseous and romm both under the "usage of undocumented apis" (and me under recommending undocumented apis)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’m thinking we still flag it and if Gaseous and Romm (and others) are using internal API’s we should try to transition to public ones.
Current versions of Gaseous aren’t using internal API’s so I’m not affected yet.
@gantoine; what do you think from a Romm point of view?
{ | ||
visible: true, | ||
icon: '<svg xml>', | ||
displayName: "Button Name", | ||
callback: () => { | ||
console.log('Button clicked'); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be put in an array as an example of how to format it, similar to all other options
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair… I’ll throw that in.
| `customBtn1` | | | ||
| `customBtn2` | | | ||
| `customBtn3` | | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these examples or can you really only have 3 extra buttons?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current PR only supports three custom buttons. I do need to change that. I’ll throw in a note that says any extra button definitions that don’t match one of the “official” button names will be considered a custom button.
Updated documentation for the EJS_Buttons variable changes proposed in EmulatorJS/EmulatorJS#992