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

Cycle road tag hotkey #6333

Closed
wants to merge 2 commits into from
Closed

Cycle road tag hotkey #6333

wants to merge 2 commits into from

Conversation

Bonkles
Copy link
Contributor

@Bonkles Bonkles commented May 7, 2019

Add new hotkey, 'Shift + C', which cycles ways through a list of the most-used highway tags.

Only works when a single way is selected.

Here are the highway tag values we cycle through: 'residential', 'service', 'track', 'unclassified', 'tertiary'. (This list can be changed by modifying the ROAD_TYPES array in the operation).

Rules:

  1. If the way has no tags whatsoever (for example, a freshly drawn line) this hotkey will add the tag 'highway=residential' to it.

  2. If the way already has the highway tag:

    1. If the tag's value is not in the list, we set the value to 'highway=residential'.
    2. Otherwise, cycle through the list as normal.

If the user selects any other type of way, they will be told that the operation isn't permitted.

In the following gif, I:

  1. Add a new line, hit the hotkey to add the residential highway tag.
  2. Change the tags of three existing highways using repeated hotkey presses.
  3. hit the Undo key three times to unwind the changes to JSHint Fixes. Fixes (mostly by hand) for JS style & syntax problems. #2.
  4. Select a non-road way and hit the hotkey, receive an error telling me that the operation isn't supported for that.
    road_tag_hotkey

Special note on undo history: I added the 'peek' history method to support better undo history for this hotkey. If the user mashes the hotkey 47 times, we don't want to have to hit undo 47 times just to remove the single tag change they made!

Bonkles and others added 2 commits May 7, 2019 11:45
Merge upstream changes to local.
…tags. Works on untagged lines or ways already tagged with 'highway'.
@Bonkles
Copy link
Contributor Author

Bonkles commented May 7, 2019

cc @gaoxm @chrisklaiber

@BjornRasmussen
Copy link
Contributor

BjornRasmussen commented May 8, 2019

This seems like a good way to speed up mapping, but there are a couple of reasons to not merge:

  1. with v3, iD will let you select feature type before drawing, so by favoriting residential, service, secondary, track, and others, drawing whatever you want would start with pressing a key from 1 to 9. After drawing, the preset wanted would already have been applied, saving time and making this pull request's shortcut unnecessary.
  2. it's not very discoverable or intuitive
  3. it's not customizable

@Bonkles
Copy link
Contributor Author

Bonkles commented May 8, 2019

Understood. Re: point 1, I know that big changes are afoot for v3. This was an attempt to get something in at the tail end of the 2.x timeframe.

  1. How can I make this more discoverable/intuitive? Adding it to the walkthrough would only educate a small portion of the new mapper population.

  2. Happy to take suggestions on how to make this more customizable!

@gaoxm
Copy link
Collaborator

gaoxm commented May 11, 2019

Following @BjornRasmussen 's concerns above:

  1. It seems the usefulness of this feature could be mostly limited to before iD v3. It could still be useful if the user wants to change the road type in v3, but that may not happen frequently.

  2. Per discoverability, the hotkey is added to the "Keyboard Shortcut" window in this PR. Ideally it would be nice to have it in the walk-though, but that would take more time and may not be worthwhile given the usefulness limitation in 1, and may possibly delay release of 2.15...

@bhousel @quincylvania let us know what your thoughts are. We're open to either
(1) not merge this in
(2) expand the list of supported road types, and potentially add the feature to walk-through in a following PR

@quincylvania
Copy link
Collaborator

@gaoxm @Bonkles Thanks for this PR! I do think it'd be better to wait and see how we can integrate the feature with iD v3. Perhaps we make this customizable via the ribbon, so the hotkey cycles through your favorite presets? v3 will also include better assistance, so we can better explain and teach features like this.

@Bonkles
Copy link
Contributor Author

Bonkles commented May 13, 2019

@quincylvania no problem whatsoever! Thanks for considering it. I'll try to stay on top of the current goings-on with iD v3 and resubmit this PR's basic functionality under the new v3 crop of changes.

@Bonkles Bonkles closed this May 20, 2019
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