Skip to content
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

Improve error messages when Rapid editor fails to load #2441

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

jake-low
Copy link
Contributor

@jake-low jake-low commented Sep 18, 2024

Previously if Rapid failed to load from the CDN, a user would see a message that the Rapid editor didn't support their browser, which was confusing. The text was also rendered inside the iframe, so it displayed in the browser default style (black, serif), which is hard to read against MapRoulette's dark green background.

This PR changes the way the iframe initializes. The parent now has to call iframe.contentWindow.setupRapid(), which returns a promise. The promise normally resolves with a Rapid editor context. If it rejects, it provides an error message that tells the user what went wrong. The RapidEditor component has been adjusted to call this method and display any error that it returns to the user. I've made sure to return different error messages in the case where Rapid fails to load from the CDN vs when it loads but then detects that the user's browser is not supported.

I've also changed the way Rapid's JavaScript bundle and CSS stylesheet are loaded, in order to deduplicate the occurrences of the Rapid CDN URL to a single constant definition. This will make it easier to implement #2438, although there are still some more problems to solve before that can be completed (I left a source code comment with a bit more details on this).

@jake-low jake-low requested a review from CollinBeczak October 2, 2024 20:26
@CollinBeczak
Copy link
Collaborator

this looks good, if you want to move forward with the environment variable implementation, you should be able to do this: jlow/rapid-editor-better-errors...CollinBeczak/rapid-editor-better-errors

@jake-low
Copy link
Contributor Author

jake-low commented Oct 3, 2024

Thanks. I considered that approach but decided to leave things simple for now, since long term I'm still working on migrating away from using react-scripts / Create React App, which would give us more flexibility in where we can use compile-time constants.

@CollinBeczak
Copy link
Collaborator

makes sense, than yeah, looks good to me!

@jake-low jake-low merged commit 1c5d86b into main Oct 3, 2024
5 of 6 checks passed
@jake-low jake-low deleted the jlow/rapid-editor-better-errors branch October 3, 2024 17:31
@CollinBeczak CollinBeczak mentioned this pull request Oct 15, 2024
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.

2 participants