Skip to content

Modify isBbox regex to allow whitespace #18

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 1, 2025

Conversation

trevorgerhardt
Copy link
Contributor

This change allows optional whitespace at the beginning, end, and around numbers, ensuring that valid bbox strings are matched even if pasted with spaces.

Background: I pasted in a bbox string with whitespace which failed to match the original regex. The subsequent JSON.parse method failed and logged the issue to the console (maybe a separate issue to address?).

This change allows optional whitespace around numbers and commas, ensuring that valid bbox strings are matched even if pasted with spaces.

Background: I pasted in a bbox string with whitespace which failed to match the original regex. The subsequent `JSON.parse` method failed and logged the issue to the console (maybe a separate issue to address?).
@bdon
Copy link
Member

bdon commented Mar 31, 2025

Could we show a better error message via popup, etc to hint what a well-formed bbox is?

Wrap `loadTextArea` in a `try {...} catch (e) {...}` to handle parsing errors for bboxes and GeoJSON. Show the textarea as invalid with a red border when there is a parsing error. Display the error text below the <textarea>.
@trevorgerhardt
Copy link
Contributor Author

Certainly. The main issue is that if the regex fails, the method then attempts to parse it as GeoJSON and when that fails the error is thrown. So it's not obvious that it was a bad bbox or bad GeoJSON. But we can make a guess.

In b333b96 I have implemented an error handler around loadTextArea that catches the errors, logs them to console, and then makes a guess for which type of error it might be:

  • SyntaxError from JSON.parse, show bbox helper text
  • Otherwise show GeoJSON advice

Example SyntaxError state:
Screenshot 2025-04-01 at 09 31 24

Additional options requiring more changes but might make for a better UX or easier, more robust error handling:

  1. Split up the <textarea> into separate "Paste bbox" and "Paste GeoJSON" textareas with separate load buttons and handling.
  2. OR separate just the button into two "Load bbox" and "Load GeoJSON" buttons that use the same textarea but separate parsing and error handling.
  3. Add https://github.com/placemark/check-geojson in order to do a more thorough error checking when parsing GeoJSON.

@bdon bdon merged commit b557505 into SliceOSM:main Apr 1, 2025
1 check passed
@bdon
Copy link
Member

bdon commented Apr 1, 2025

Thanks, it's live now on https://slice.openstreetmap.us

additional PRs to make the error handling better are welcome

@trevorgerhardt trevorgerhardt deleted the patch-2 branch April 30, 2025 08:14
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