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

Role value does not persist after user tabs off combobox #5449

Closed
mboeringa opened this issue Oct 30, 2018 · 15 comments
Closed

Role value does not persist after user tabs off combobox #5449

mboeringa opened this issue Oct 30, 2018 · 15 comments
Labels
bug-release-blocker An important bug - let's get this fixed in the next release!

Comments

@mboeringa
Copy link

mboeringa commented Oct 30, 2018

I only recently started adding turn restrictions using iD, but have noticed a disturbing issue. In the current setup, when you add a turn restriction, the textline for setting the role is editable: it allows typing. It even seems to support auto-completion.

As an advanced user will start to type the role, instead of waiting for any context menu to popup, iD suggests to support this, see the image:

afbeelding

However, confusingly, once you continue adding members (or at least think so!), and finally click the turn relation's link, you discover that while the members have been added, none of the roles have actually been set! If a user uploads the data like this, which is perfectly possible as iD doesn't block this, you end up with an incomplete and broken turn restriction relation in the database.

afbeelding

It was only after several tries, that I actually realized iD doesn't support setting roles via typing, but that you must wait for the context / role drop down menu with the preset roles to popup, before you can truly set the roles. Only roles chosen from the turn relation drop down preset, will actually be set on a turn restriction relation.

afbeelding

This is really confusing. I really suggest to dissalow editing and typing in the textbox, and possibly also to disallow uploading incomplete turn restriction relations to avoid polluting the OSM database with broken turn relations.

@bhousel
Copy link
Member

bhousel commented Oct 30, 2018

I will check this - it sounds like the autocomplete is just running slow.

However I do encourage you to add turn restrictions in iD using the dedicated turn restriction editor, and not by manually editing the relations.

@mboeringa
Copy link
Author

After looking through some more issues, I realize this is related or duplicate of #4900, that seems to describe the same issue for route relations (it may be a general issue with any relation type having preset roles).

@mboeringa
Copy link
Author

However I do encourage you to add turn restrictions in iD using the dedicated turn restriction editor, and not by manually editing the relations.

I actually start any new turn relation by clicking the + button for adding new relations, and then choosing Turn Restriction from the available options.

@mboeringa
Copy link
Author

mboeringa commented Oct 30, 2018

I will check this - it sounds like the autocomplete is just running slow.

@bhousel : What pleads against this, is that when I continue typing, and type all individual letters / charachters of the role (e.g. "from"), and then click in another part of the editing window like in the tags for setting a tag, then the text I typed continues to be shown, and suggests to have been "accepted" by iD. Only when I go back to the actual turn relation to see its members and roles, I discover that nothing has been set.

@bhousel
Copy link
Member

bhousel commented Oct 30, 2018

I actually start any new turn relation by clicking the + button for adding new relations, and then choosing Turn Restriction from the available options.

A much easier way to work with turn restrictions is to select any road junction and use the turn restriction editor in the sidebar. You can select any incoming road and inspect and change the restrictions for any path through the intersection. It has some nice built in help too.

turn restrictions

@mboeringa
Copy link
Author

@bhousel , this is important: I consistently see this fail in Microsoft Edge. I have now tried this in Firefox 63.0, and it doesn't have the same issue.

@mboeringa
Copy link
Author

mboeringa commented Oct 30, 2018

A much easier way to work with turn restrictions is to select any road junction and use the turn restriction editor in the sidebar.

Thanks for the suggestion, but I don't see this specific relation editor popup when I click on the connection point between two roads?... Is there anything else I need to do?

@bhousel
Copy link
Member

bhousel commented Oct 30, 2018

Thanks for the suggestion, but I don't see this specific relation editor popup when I click on the connection point between two roads?... Is there anything else I need to do?

Not sure - what roads does it not show for? Can you share the location?

@bhousel bhousel added the bug-browser-specific A bug that only appears in certain browsers label Oct 31, 2018
@mboeringa
Copy link
Author

mboeringa commented Nov 14, 2018

@bhousel

Just curious, but were you able to reproduce the originally reported issue in Microsoft Edge?

And note that the probably related issue #4900 also shows Safari 11.1 on macOS 10.12.6 being affected. The OP who reported that issue also confirmed my observation that Firefox is OK, and doesn't show the same issue ("but not with Firefox 59.0.2 on macOS"): #4900 (comment)

@bhousel
Copy link
Member

bhousel commented Nov 14, 2018

Just curious, but were you able to reproduce the originally reported issue in Microsoft Edge?

No, sorry I haven't looked into this yet.

@bhousel
Copy link
Member

bhousel commented Dec 12, 2018

I made some changes that I think should fix this.. Could you test in http://preview.ideditor.com/master/ and let me know if changing the role sticks now?

@mboeringa
Copy link
Author

mboeringa commented Dec 12, 2018

@bhousel

Unfortunately, this still fails in Microsoft Edge browser when tested through that preview link you provided. The roles still don't stick when I type them instead of selecting them from the drop down menu.

I really think if you cannot get the roles to stick in iD, then the easiest solution would be to disallow typing in the role textbox, and thereby forcing users to select the role from the drop-down role menu (which will cause the roles to stick if selected this way in Edge).

Keeping this issue unfixed, will likely mean Edge and Safari users will unconsciously upload broken turn restriction relations without proper roles set, which is very undesirable.

afbeelding

@bhousel bhousel reopened this Dec 12, 2018
@bhousel bhousel changed the title Disallow typing in "role" textbox of turn restriction relation, editing causes issues / empty roles Role value does not persist after user tabs off combobox - MS Edge only? Dec 12, 2018
@bhousel bhousel added bug-release-blocker An important bug - let's get this fixed in the next release! wip Work in progress and removed bug-browser-specific A bug that only appears in certain browsers labels Dec 12, 2018
@bhousel
Copy link
Member

bhousel commented Dec 12, 2018

I figured out what was going on with this one. Fix coming soon..

@bhousel bhousel changed the title Role value does not persist after user tabs off combobox - MS Edge only? Role value does not persist after user tabs off combobox Dec 12, 2018
@bhousel bhousel removed the wip Work in progress label Dec 12, 2018
@bhousel
Copy link
Member

bhousel commented Dec 12, 2018

@mboeringa can you test this again in http://preview.ideditor.com/master/ ?
I think it's really fixed this time.

The issue was that some browsers don't always send a change event for changes that the user didn't do themselves (in this case, the autocomplete was doing the change). We have workarounds all around the rest of the code to listen for both change and blur and see what actually changed - I just needed to add similar code to the role field too.

@mboeringa
Copy link
Author

mboeringa commented Dec 12, 2018

@bhousel

I think you nailed it! The roles seem to stick now when typed in Edge. Going back and forth between relation members and the relation itself, the roles seem consistently kept.

It would be good to double check in Safari though, with which I can't help you.

Thanks for the work on this!

afbeelding

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-release-blocker An important bug - let's get this fixed in the next release!
Projects
None yet
Development

No branches or pull requests

2 participants