AR-js-org
Replaced alert method for webcam errors
Adjusted webcam error reporting to insert a DOM object instead of native alert method
⚠️ All PRs have to be done versus 'dev' branch, so be aware of that, or we'll close your issue ⚠️
What kind of change does this PR introduce?
Improvement
Can it be referenced to an Issue? If so what is the issue # ?
How can we test it?
Run an AR.js project and reject giving the permission to use the camera. No alert() box should be shown.
Summary
The usage of the native alert() function does not allow to control the UX flow nor the look and feel of the error message. With the change, a DOM element is attached to the document, containing the error.
Does this PR introduce a breaking change? The side-effect of appending a DOM element could be unwanted. However, three.js/src/threex/arjs-source.js also adds such a DOM element with id #error-popup and, therefore, clients should have designed for it already.
Please TEST your PR before proposing it. Specify here what device you have used for tests, version of OS and version of Browser
Other information
Looks good, thanks! Agree about using a div being better than an alert.
I note you repeat the same code twice, could you put that in a function? Thanks.
@salam @nickw1 i have nothing against this PR as Nick said we could maybe avoid repeating the code, and CI is failing probably because you haven't run the prettier command and the build command. You should follow these steps:
//to be sure that all dependencies are installed run the install command
npm install
// then check if your code is well formed
npm run format-check
// if fails run the format command
npm run format
// then run the build script ( for a prodcution build)
npm run build
// for a development build
npm run build:dev
@salam have finally merged this, apologies for the delay, have some free time at the moment so going back over still-open PRs.