[openstreetmap/openstreetmap-website] Hide changesets from history list that don't fit into view (PR #3417)

kallejre notifications at github.com
Wed Jan 12 01:26:08 UTC 2022


Not full but partial fix for issue #3242. While the original issue suggested to use a slider for filtering changesets based on size, i encountered some issues with trying to implement [version with a slider](https://github.com/openstreetmap/openstreetmap-website/issues/3242#issuecomment-1006709194) and decided to use variant with simple checkbox.

Those problems revolved mostly around having bit too large impact to be made without getting maintainers' opinion, such as adding multiple extra parameters for API, defining the extremes of the slider and how it should transition from one to another. In other words, what should be effect of setting slider to one end, to other and and how it behaves at any intermediate step. 
There were also some usability issues (some mobile devices didn't work smoothly with slider) and fears about having too severe performance impact on server. One of considered solutions (discussed outside of github) was to calculate area of every changeset and compare bbox coverage percentages, but that would have been computationally too expensive.

This PR adds following:

- Checkbox on `/history` pages allowing to hide all changesets that don't fully fit into current view.
- Parameter `fit_bbox` (boolean) for both API and history listing backends.
- Related test and i18n localization.
- Function `check_boolean` intended to be reused for parsing parameters `opened` and `closed`
- Fix for history view's bounding box creation on low zoom levels - previously use of Leaflet's `.wrap()` method on bbox caused bounding boxes' latitudes to be cropped too small (e.g if zoom to lvl 1 zoom and resize window in a way that left and right edge of map match, then only changesets from that gap would be shown).

Here's image of how new history page looks like:
![osm-animated](https://user-images.githubusercontent.com/22601238/149046209-105204c5-c37c-400b-a8c9-514c3815b017.gif)

I think i should mention that the checkbox is unchecked (`false`) by default and API also treats missing `fit_bbox` parameter as `false`. This also tries to cater for concerns about users possibly filtering out some local changesets, trying to ensure no-one accidentaly hides changesets by leaving old behaviour as default action.

PS. Weakly related question from my original comment on the issue, slightly rephrased:
> Probably I don't know how Ruby works because i have never used it, but shouldn't API `changesets` endpoint also support URL parameters `open/closed=false`? Currently API seems to only check if parameter is present, not it's value. Therefore code from that part is probably not reusable for the changeset filtering methods. Was such decision to ignore parameter values bug or a feature?
You can view, comment on, or merge this pull request online at:

  https://github.com/openstreetmap/openstreetmap-website/pull/3417

-- Commit Summary --

  * Add back-end support for fitting changesets into bbox
  * Fix issue where bbox had wrong width at low zoom levels
  * Basic frontend support for hiding global changesets
  * Fixed jQuery trying to access non-existent element
  * Merge branch 'openstreetmap:master' into master
  * PR compliance/maintenance

-- File Changes --

    M app/assets/javascripts/index/history.js (10)
    M app/controllers/api/changesets_controller.rb (27)
    M app/controllers/changesets_controller.rb (29)
    M app/views/changesets/history.html.erb (5)
    M config/locales/en.yml (1)
    M test/controllers/changesets_controller_test.rb (13)

-- Patch Links --

https://github.com/openstreetmap/openstreetmap-website/pull/3417.patch
https://github.com/openstreetmap/openstreetmap-website/pull/3417.diff

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3417
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/3417 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20220111/b4f9876a/attachment-0001.htm>


More information about the rails-dev mailing list