-
Notifications
You must be signed in to change notification settings - Fork 580
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
simulator extensions: apply aspect ratio to simx iframe #10222
Conversation
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.
Just a question about the regex, but otherwise looks good
pxtsim/runtime.ts
Outdated
// Copied from pxtlib/util.ts | ||
export function sanitizeFileName(name: string): string { | ||
/* eslint-disable no-control-regex */ | ||
return name.replace(/[()\\\/.,?*^:<>!;'#$%^&|"@+=«»°{}\[\]¾½¼³²¦¬¤¢£~¯¸`±\x00-\x1F]/g, '').trim().replace(/\s+/g, '-'); |
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.
Two questions for this regex. Is there not a way to capture all special characters without something like this? Also, how were you able to type some of these?
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 method was copy/pasted from pxtlib/util.ts, because that module is not available in the simulator. I don't know the history of this regex, but wanted to use it for consistency.
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 agree the regex could be written a lot better. It should take the approach of defining what is allowed rather than what is not.
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.
Your comment made me realize that css has slightly different validation rules than filenames. css names can't start with a number, for example. Pushed an update to fix this.
* apply simx aspect ratio * lint * sanitizeCssName
* Support for 3rd-party simulator extensions (#10213) * support for simulator extensions * lint * lint * n * restore * restore * read * wording * simxdevmode * add test simx * async componentDidMount * `e` -> `x` * Add simulator extension doc page (#10219) * support for simulator extensions * lint * lint * n * restore * restore * read * wording * simxdevmode * add test simx * async componentDidMount * `e` -> `x` * update docs * simulator extensions: apply aspect ratio to simx iframe (#10222) * apply simx aspect ratio * lint * sanitizeCssName * Fix simx path formatting issue (#10229) * fix path formatting issue * simpler fix * restore comment
aspectRatio
, but it was getting ignored. ModifiedstartSimulatorExtension
method to take anaspectRatio
parameter and apply it to the simulator extension's iframe.