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 a parent ways section to the inspector for vertexes, and improve vertex navigation #3631

Closed
wants to merge 7 commits into from

Conversation

slhh
Copy link
Contributor

@slhh slhh commented Dec 4, 2016

This adds a section to the inspector which shows the connected ways of a vertex #3603 .
It enables the user to select a specific way in case of coincidence, which makes it a partial solution for #1239.
It provides an visual equivalent of the keyboard vertex navigation, which can also make the respective shortcuts discoverable.

In addition the vertex keyboard navigation has been improved.

This pull request is currently a first preview version for discussion.

Known issues:

  • npm test does still fail on a few tests, but I have the same issue with the current master (on Windows 10).
  • The visual indications of the current related parent in the parent ways section of the inspector does not update as long as the selected element is not changed. Update of the sidebar seems to be prohibited when mode select is reentered without changing the selectedIDs. @bhousel How to address this? Update the sidebar more often, or implement some callbacks for updating the related parent?
  • some CSS styling to be done
  • preparations for translation to be done

@bhousel
Copy link
Member

bhousel commented Dec 8, 2016

It's going to be a few days before I can do a thorough review.. To help me out, can you rebase to master and squash away all the merge commits?

to your points:

npm test does still fail on a few tests, but I have the same issue with the current master (on Windows 10).

Yes I noticed this too 😞 If you have time, it would be awesome if you could look into the failing tests on Windows and make a separate PR for it. I think it's something to do with PhantomJS and xmlhttprequest and maybe side effects from testing the osm service leaving the xmlhttprequest in a bad state. 👈 stuff like this is more important than adding new features to iD.

The visual indications of the current related parent in the parent ways section of the inspector does not update as long as the selected element is not changed. Update of the sidebar seems to be prohibited when mode select is reentered without changing the selectedIDs. @bhousel How to address this? Update the sidebar more often, or implement some callbacks for updating the related parent?

There is no obvious way that I can think of. You're right that the sidebar wouldn't normally redraw unless a tag is changed or a different feature is selected. It sounds like this new sidebar component would need to listen to an event dispatched whenever "current related parent" is changed by the user. Maybe we don't need that feature? Could it just display all the related parents? You might be able to indicate which one is current with css.

@slhh
Copy link
Contributor Author

slhh commented Dec 16, 2016

@bhousel I have rebased it. The windows build issue seems to be a too hard one for me due to missing knowledge of such web server things.

I have managed to get the related parent display updated by css.

I have also added support for relations in view of keyboard navigation. This requires autoloading of selected elements which are not already in context. This is included here, but results in failure of one of your recently added tests. ModeSelect does no longer reject invalid ids, but does return to to modeBrowse after receiving the callbacks of loadEntity. I think your test might just wait some time before checking the mode, but I don't know how to do it in the test code.

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.

None yet

3 participants