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

Fix UDF choice editing helper function #3259

Closed
wants to merge 15 commits into from
Closed

Conversation

jwalgran
Copy link
Contributor

@jwalgran jwalgran commented Nov 2, 2018

Overview

In the case where choice fields contain numeric values, this helper is called with a number and fails unless we force the value to a string.

Connects https://github.com/OpenTreeMap/otm-clients/issues/441

Testing Instructions

screen shot 2018-11-02 at 10 35 46 am

* Refresh the page and click `Edit`. Start typing a new choice value and you will see an exception in the console

screen shot 2018-11-02 at 10 38 45 am

* Checkout this branch, reload the page, and click `Edit`. * Verify that you can successfully add a new choice value and save it.

jwalgran and others added 15 commits May 24, 2018 14:21
Replaces use of `urllib` and `urllib2` with Requests library while
attempting to maintain backwards compatibility.
Adds a command to write out the feature id, tree id, and the URL of all photos
for an instance as a CSV. This CSV can then be processed on a developer
workstation to export all of the instance photos.
Add create_photo_csv management command
Tapping on the layers control in Mobile Safari was not opening the layer
selector. I was not able to reproduce this issue in a separate test application,
only within OTM.

After much trial and error I discovered that changing this line
  L.DomEvent.on(el, L.Draggable.START.join(' '), stop);
to
  L.DomEvent.on(container, 'mousedown', stop); in
DomEvent.disableClickPropagation resolved the issue. Effectively, this
prevents the `touchstart` event from being stopped.

I opted to implement this as a runtime patch applied within MapManager instead
of a global patch to the Leaflet source because there are no other Leaflet
behaviors that are known to be broken in Mobile Safari and I did not want to
risk introducing a regression in some other event handling.

I minimized the chance of forgetting about this patch if/when we upgrade our
version of Leaflet by adding a comment to package.json.
…ector

Monkey patch the Leaflet layer selector to work in Mobile Safari
Removes the configuration file used by Circle CI.
Copied from https://docs.rollbar.com/docs/browser-js

Attempts to remove the "Telemetry requires the latest release of rollbar.js."
message when viewing logged errors on the Rollbar dashboard.
Addresses the following error raised by Chrome when viewing an embedded map:

    SecurityError: Failed to read the 'localStorage' property from 'Window':
    Access is denied for this document.
Update Rollbar JS client and catch localStorage security error
In the case where choice fields contain numeric values, this helper is called
with a number and fails unless we force the value to a string.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 83.796% when pulling 3604b8d on jcw/fix-udf-helper into cb1027c on develop.

Copy link
Contributor

@mmcfarland mmcfarland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tried on a variety of value types and does not generate an error. Side note, that's a really nice interface for adding choices.

@mmcfarland mmcfarland assigned jwalgran and unassigned mmcfarland Nov 5, 2018
@jwalgran
Copy link
Contributor Author

jwalgran commented Nov 6, 2018

Thanks. I am going to retarget this to the newly created hotfix/2.21.4 branch.

@jwalgran jwalgran changed the base branch from develop to hotfix/2.21.4 November 6, 2018 16:11
@jwalgran jwalgran closed this Nov 6, 2018
@jwalgran jwalgran changed the base branch from hotfix/2.21.4 to master November 6, 2018 16:22
@jwalgran
Copy link
Contributor Author

jwalgran commented Nov 6, 2018

Dealing with an issue retargeting this to the hotfix branch. Will reopen.

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.

4 participants