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

Add requesting review #4223

Merged
merged 32 commits into from
Aug 17, 2017
Merged

Add requesting review #4223

merged 32 commits into from
Aug 17, 2017

Conversation

kepta
Copy link
Collaborator

@kepta kepta commented Aug 8, 2017

This adds review request.

@pnorman
Copy link
Contributor

pnorman commented Aug 8, 2017

It'd be good to sort the tagging out with the community, not on the iD issue tracker.

@bhousel
Copy link
Member

bhousel commented Aug 8, 2017

It'd be good to sort the tagging out with the community, not on the iD issue tracker.

Why? My feeling has always been that:

  1. The iD issue tracker is part of the community. It's open, and anybody who wants can follow what we do or comment or suggest changes.
  2. Changeset tags are really more decided by the editor software than by anybody else.
  3. We can use any tags we like.

I am really happy to change the tag, if anybody cares. It doesn't have to be review_requested=yes.

(doesn't really add anything and not written for newbies)
uiCommit is getting kind of big as we add more to the commit pane.

I'm going to split it up and put the field rendering code into a separate
module, similar to how uiEntityEditor embeds uiPresetEditor for the fields.

This allows us to add a few more fields that users can set on their changesets
(like hashtags, source), and even hide them under a "Add field" dropdown.
@iandees
Copy link
Collaborator

iandees commented Aug 11, 2017

I realize I could check this out and look, but what are y'all thinking about for prompting the user to ask for review? A popup? A checkbox near the changeset save button?

@bhousel
Copy link
Member

bhousel commented Aug 11, 2017

This request review field is great and I could merge it right now, but instead I'm going to spend some time to refactor the commit pane a little bit - it is like 500 lines of code and getting difficult to manage as we add things to it.

Managing a bunch of fields and the tags that they change is something that uiEntityEditor and uiPresetEditor already do well, so I'm going to use that as a model to split uiCommit into smaller modules.

@iandees, here's a screenshot showing current placement of the checkbox, along with some dummy fields that I stuck at the top. The dummy fields are being rendered by the new uiChangesetEditor component.

screenshot 2017-08-11 17 29 02

@bhousel
Copy link
Member

bhousel commented Aug 14, 2017

Refactoring the commit pane by using the new uiChangesetEditor module is working nicely. It now creates real uiField objects and supports a "more fields" section. We can let users tag a source now. Here are some things that I need to put back:

  • dropdown for previous changesets
  • placeholder texts
  • google warning
  • enable save button when comment exists
  • double check the warnings and summary sections of uiCommit

and some scope creep..

@bhousel
Copy link
Member

bhousel commented Aug 14, 2017

Looks like this currently

save mode

@iandees
Copy link
Collaborator

iandees commented Aug 14, 2017

It would be great to think of a way to call out that "request review" checkbox, especially for new folks. Can it pulsate or be a different color or have an arrow with a friendly globe suggesting they give it a try?

@bhousel
Copy link
Member

bhousel commented Aug 14, 2017

It would be great to think of a way to call out that "request review" checkbox, especially for new folks. Can it pulsate or be a different color or have an arrow with a friendly globe suggesting they give it a try?

I have thoughts on this request review checkbox and how it might be perceived by different kinds of users.

Not everybody wants feedback on their edits. For example, millennials may be more likely to want the feedback, and genx'ers may be more likely to not want the feedback. Majority users may be more comfortable with public edits, and minorities may not feel comfortable having their edits under additional scrutiny.

I think this feature could be very helpful and welcome to a lot of people - especially new users - but I don't want to put assumptions into the software, and so that's why we ask the user whether they want the review or not.

Also at this point we don't know how many people will check the box, or who will step up to do the reviews. The answer is some combination of - OSMCha, whodidit, bots, rss feeds, dedicated validators, paid mapping teams, and volunteers.

I thought about just checking the box if it's a user's first edit, or having the preference be "sticky", but I don't want to promise something the OSM community can't deliver. So for now, let's leave the box unchecked, and "opt-in" to review. We can change this later and encourage more users to check the box, if after a few months we decide that the reviews are working well and helpful.

@mvexel
Copy link
Contributor

mvexel commented Aug 14, 2017

Agree with @bhousel -- until we know how many requests will actually be honored it's probably better to not call it out too much. If people don't want to review or use discouraging words to do so it would just serve to turn more people off than the other way around.

@talllguy
Copy link

Here's an idea for frequently used sources as checks, streamlining taginfo a bit
image

@bhousel
Copy link
Member

bhousel commented Aug 15, 2017

Hey @talllguy, one benefit of the recent refactoring work is that it makes it super easy to just reuse fields like the ones that iD already has other places. We don't have a field that's an array-of-checkboxes, but we do have a multiselect field that turns the values into a semicolon separated list.

What do you think of this?:

sources

@talllguy
Copy link

talllguy commented Aug 15, 2017

@bhousel I like to see this fields reused this way. This implementation looks cool! I'd use this.

Next steps would be using an algorithm to analyze the changeset comment and other metadata to suggest source tags. ;)

@bhousel
Copy link
Member

bhousel commented Aug 16, 2017

FYI just pushed a working implementation of hashtags (see #2834).
This is just another semicolon-delimited multiselect field, and I added some extra code to detect hashtags embedded in the comment.

The idea here is to give mapathons, task managers, QA tools, a special field to store their hashtags, instead of (ab)using the comment field, which some people don't like. Hashtags will also be settable through the API just like comments currently are.

hashtags

(it will say "0" for someone making their first edit)
These tags all start with `ideditor:`
(closes #3968)

```
ideditor:walkthrough_completed=yes
ideditor:walkthrough_progress=welcome;navigation;point;area;line;building;startEditing
ideditor:walkthrough_started=yes
```
@bhousel bhousel merged commit 0ea043b into master Aug 17, 2017
@bhousel bhousel deleted the request_review branch August 17, 2017 16:15
@bhousel
Copy link
Member

bhousel commented Aug 17, 2017

Just merged this - thanks for your patience!
Hope these new features will make it easier for other projects to analyze changesets.

simon04 pushed a commit to JOSM/josm that referenced this pull request Sep 4, 2017
floscher pushed a commit to floscher/josm that referenced this pull request Sep 4, 2017
floscher pushed a commit to floscher/josm that referenced this pull request Sep 14, 2017
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.

7 participants