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

Text raw tag editor / Copy-paste tags #6302

Merged
merged 12 commits into from
May 3, 2019
Merged

Text raw tag editor / Copy-paste tags #6302

merged 12 commits into from
May 3, 2019

Conversation

bhousel
Copy link
Member

@bhousel bhousel commented May 3, 2019

This change adds ability to toggle between the list view and a text view of tags, so that people can make faster tag edits or copy/paste tags.

closes #6185
closes #839

@bhousel
Copy link
Member Author

bhousel commented May 3, 2019

It works like this ✨

text tag editing

.merge(textarea);

textarea
.attr('rows', rowData.length + 1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we disable spell checking and autocorrect? Safari’s autocorrect is pretty aggressive inside textareas, and the 〰️ underlines get pretty annoying with raw tags.

Copy link
Member Author

Choose a reason for hiding this comment

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

I dunno sometimes it does the right thing :trollface:

Screenshot 2019-05-03 10 09 51

@tordans
Copy link
Collaborator

tordans commented May 3, 2019

Does iD have code already that is able to resize the height of an textarea based on its content? (Like https://css-tricks.com/textarea-tricks/#article-header-id-6) I would not add it just for this element (it's quite a bit of JS just for this**), but if it only needs to be "activated" for this textarea, it would be nice for the last bit of the GIF where the text area has a scrollbar.
Bildschirmfoto 2019-05-03 um 07 08 27
**) The <div contenteditable> approach from the article above might be a less-code solution that works in this case; also maybe for the autocorrect issue?

@sun-geo
Copy link
Contributor

sun-geo commented May 3, 2019

Wow, happy to see any progress for (bulk) copy/paste function :-)

Yes, maybe the window height could be increased a litte bit same time while when pasting the new lines? At that moment:
y-window-height

@bhousel
Copy link
Member Author

bhousel commented May 3, 2019

This is good enough to merge now..
Please try it out in http://preview.ideditor.com/master/ and open any followup issues that you find!

@bhousel bhousel merged commit ddc9d16 into master May 3, 2019
@bhousel bhousel deleted the text-raw-tag-editor branch May 3, 2019 18:53
@mmd-osm
Copy link
Contributor

mmd-osm commented May 3, 2019

Some folks add new line characters in the description: https://www.openstreetmap.org/way/463013906

Would this work as is?

@bhousel
Copy link
Member Author

bhousel commented May 3, 2019

Some folks add new line characters in the description: https://www.openstreetmap.org/way/463013906

Would this work as is?

Yep, that's handled in 16ec257
I'm using JSON.stringify to escape newlines and other unprintable characters.

@mmd-osm
Copy link
Contributor

mmd-osm commented May 3, 2019

ok, looks good, newline got replaced by an \n

I found one issue with two rows having the same key: one of them gets silently discarded when I switch back to the list.

sport=soccer
sport=rugby

Merging them into one entry with semicolon as a separator might be an option, dunno.

@quincylvania
Copy link
Collaborator

I found one issue with two rows having the same key: one of them gets silently discarded when I switch back to the list.

I think the preferred behavior would be to overwrite the old value, if any.

@bhousel
Copy link
Member Author

bhousel commented May 3, 2019

I think the preferred behavior would be to overwrite the old value, if any.

It just keeps whichever is the last one in the list. I'm ok with this, since I'd expect people to paste their new values at the end of the list.

@sun-geo
Copy link
Contributor

sun-geo commented May 5, 2019

Maybe change the blue and grey colour:
image

(blue for clickable items, like "zoom to this" / "All fields" / "All tags")?

@bhousel
Copy link
Member Author

bhousel commented May 5, 2019

Maybe change the blue and grey colour:
(blue for clickable items, like "zoom to this" / "All fields" / "All tags")?

Oh yeah, I was thinking about this too. I do like using a consistent blue for links. Maybe I could make it look more like these toggles we use elsewhere:

toggle

@sun-geo
Copy link
Contributor

sun-geo commented May 5, 2019

yes.

Or (if there is not enough space) just one blue button, with remaining blue colour, but changing list icon <=> cursor icon? I'm not sure...

edit: or something like a switch(blue of course):
image

@quincylvania quincylvania added this to the 2.15.0 milestone May 6, 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.

Allow users to paste several "k=v" tag pairs into the raw tag editor Copy-paste tags
6 participants