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

In-browser place editor off master #620

Merged
merged 414 commits into from
May 15, 2017
Merged

In-browser place editor off master #620

merged 414 commits into from
May 15, 2017

Conversation

goldpbear
Copy link
Contributor

@goldpbear goldpbear commented Feb 9, 2017

Addresses: #401, #410, #438, #497, #498, #499, #500, #501, #523, #525.

Not ready to merge yet

Features added:

  • Place detail view editor mode for authenticated users
    • By default, edit mode will make it possible to edit form fields that were filled out by the submitter as well as form fields for which the submitter provided no response
  • Geometry editing capabilities
    • Geometry editing support is provided by a new form field type called geometryToolbar. If a category has a form field of this type in it, geometry editing functionality will be available
  • Rich text support via Quill
    • To make a form field rich text-enabled, use the type: richTextarea flag in the field's configuration
  • Embedded image uploads
    • Clicking the Quill image button will trigger an attachment upload and will insert an <img> tag inside the rich text field with the src attribute set to the uploaded image url
    • Note that images attached via Quill are just regular place model attachments. This means that they will be displayed above any form fields in a place detail view, as this is the default behavior for places. To prevent this from happening, use the suppressAttachments: true flag in a category's config
  • Image manipulation
    • Embedded images can be resized by clicking on them and dragging resize handles
    • Embedded images can be moved by dragging them within the Quill interface
  • Localstorage-based "draft mode" for automatically saving in-progress editor changes client side
  • Admin-only form categories and admin-only form fields
    • To make a category or form field admin-only, add the following keys to either the category config or the field config, as appropriate:
      admin_only: true
      admin_msg: _((For administrator use only))
      
      Note that currently admin-only categories and fields are admin-only strictly at the UI level: because new places generate POST requests, there are no corresponding backend checks to ensure that submissions from admin-only categories or submissions containing admin-only fields actually come from administrators.
  • Form-only fields
    • Add a form_only: true to a form field's config to prevent the value of that field from appearing in place detail views. The value of form-only fields will still be sent to the client (unlike the values of private fields).

To enable the editor mode for a dataset:

  1. In the Django admin panel, click on Data sets and then the name of the dataset you'd like to enable editing for.
  2. Under the Groups section, create a new group called administrators if it doesn't already exist. Note that this group must be called administrators.
  3. Add users to whom you'd like to grant editor privileges by selecting the user in the Available submitters panel and clicking the right arrow to move them to the Chosen submitters panel.
  4. Click Edit permissions below the submitters panels, and grant retrieve, create, update, and destroy privileges. Also make sure that a * character is entered in the text box at left. The help text in the admin panel suggests that this box can be left blank, but this is in fact not true.
    Click Save.
  5. Now when any of the Chosen submitters are logged in via a social auth, an edit button will appear on place detail views that belong to datasets where they have edit privileges.

Note that it is possible to add new users to an administrators group even if they haven't already logged into the app via a social service. In the admin panel, click Users, then Add user +, then create a new user. The username must match the social service handle of the person you'd like to grant administrator access to. Then, in the User social auths panel, click Add user social auth +, select the user you just created under User, enter the name of the social service provider (twitter or facebook), then enter the social user's Uid. The Uid can be looked up online: here for twitter, and here for facebook.

@goldpbear goldpbear force-pushed the browserEditor-401 branch 3 times, most recently from f693204 to 7edc654 Compare February 16, 2017 06:38
Passing the reset option seems to elimiate the problem of directly-loaded
landmarks causing a portion of the map to initially not be rendered and the
center of the map to be displayed incorrectly.

Fixes: #504
Make sure we reset the dynamic form state if we navigate away from
a rendered form (say, to the list view). Otherwise, it's possible
to corrupt the form's state by returning to the form after navigating
away.
- support places with landmark-style urls
- reduce code duplication in AppView
- slight loading performance boost on direct url loads

Addresses: #527, #549
This commit adds the following functionality:

- Polygon and linestring geometry creation via the dynamic form
- Polygon and linestring geometry editing via the detail view editor
- Geometry fill and opacity control

It introduces dependencies on:

- The Spectrum jQuery colorpicker library
- The leaflet-draw plugin

Addresses: #410
- Move lib and css files out of old sa_web folder
- Fix merge errors
@goldpbear goldpbear force-pushed the browserEditor-401 branch 2 times, most recently from 501fb2a to 5a80e06 Compare March 6, 2017 03:44
@goldpbear goldpbear force-pushed the browserEditor-401 branch 2 times, most recently from 62ca4d9 to d83cf19 Compare March 16, 2017 18:20
@goldpbear goldpbear self-assigned this Apr 30, 2017
Copy link
Contributor

@zmbc zmbc left a comment

Choose a reason for hiding this comment

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

Overall, looks good. Willing to help with the README stuff if necessary.

README.md Outdated
<DATASET-ID>_SITE_URL=https://path/to/dataset/
<DATASET-ID>_DATASET_KEY=dataset_key
DATASET-ID_SITE_URL=https://path/to/dataset/
DATASET-ID_DATASET_KEY=dataset_key
Copy link
Contributor

Choose a reason for hiding this comment

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

You'll have to merge this change with the new README. Also, can you explain the thinking behind removing the angle brackets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't quite remember why we decided to remove the angle brackets, although I do remember having a discussion about it at some point. Do you think it's better to leave them in?

README.md Outdated
@@ -104,6 +104,30 @@ NOTE: If you're new to programming with virtualenv, be sure to remember to activ
source env/bin/activate
```

### Using the in-app editor

The platform includes an in-app editor (currently only available on the `develop` branch) that you can use to update and hide places and comments on a per-dataset basis. Only authenticated administrators are allowed to make edits. Authentication is performed via third-party social media services (Twitter and Facebook are currently supported), so administrators will need an account on either of these services to use the editor.
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably don't need to tell them which branches it's on, because your README change will be on those same branches.

Also, as before, you'll need to merge this into the new README. This new section should be under the Usage subheading, because its intended audience is users. Take a quick look at this to make sure you're following it.

<nav class="list-toggle-nav">
<a class="list-toggle-btn btn btn-block">
<span class="show-the-map is-visuallyhidden">{{ show_map_button_label }}</span>
<span class="is-screen-reader-text">/</span>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the / screen-reader-text?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This block of code was relocated from base.html as-is. It already had that slash, and I'm not totally sure why. Maybe just so the screen reader span has some content in it?

<!-- Template for the row of editor buttons visible at the top of place
detail views. -->

{{#if isEditable}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, but maybe this #if should be outside the template (wrapped around the template's use).

Copy link
Contributor Author

@goldpbear goldpbear May 2, 2017

Choose a reason for hiding this comment

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

I debated this as well. My reasoning for having the {{#if}} check inside the template here is that the check is integral to the way this template should behave. In other words, there's (currently, at least) no situation where we'd want to render this template without the {{#if}} check.

class="place-value place-value-{{ name }}"
alt="{{ name }}" />
<!-- {{#if ../isEditingToggled}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

resolved

- id: vision
title: _(Community Vision)
visibleDefault: false
visibleDefault: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

@@ -997,6 +1020,50 @@ place:
display_prompt: _( )
placeholder: _(Enter description...)
optional: false
- category: featured_place
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be here?

@@ -1027,8 +1096,20 @@ place:
- label: _(Other)
value: other
optional: true # NOTE: checkbox_big_button inputs have no validation
- name: reside_work_georgetown
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

@@ -1048,8 +1129,8 @@ place:
placeholder: _(Enter title...)
optional: false
- name: description
type: textarea
prompt: "_(What's your question?)"
type: richTextarea
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto

@@ -0,0 +1,8 @@

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is sa_web here at all?

@goldpbear goldpbear merged commit 80ce8f0 into master May 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants