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

Show "node also part of ways" as nested lists on way pages #5317

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

AntonKhorev
Copy link
Collaborator

@AntonKhorev AntonKhorev commented Nov 13, 2024

Way pages have lists of nodes. Every way node can be shared with other ways, and if it is, then all of those ways are also listed. Currently it's done as "(part of ways ...)" phrases with ways listed using to_sentence. Those are cumbersome to deal with. You can never be sure about grammar in translations. Also it's more difficult to style because each way may or may not have an icon, there's a space reserved for that icon, that icon has to be positioned into that space probably with a bunch of pixel values. That's what #5080 does and that's one thing I don't like about it.

In this PR I convert those phrases with inline icons into nested lists. After that icons are going to be located at the beginning of <li>s both for nested lists of ways and outer lists of nodes. This will make icons easier to position.

Also fixes #1147. Inner lists of ways contain only unique ways to avoid #1147 (comment). The outer list of all nodes has details collapsed for any node that was already written (see node 2071 on screenshots below).

Before:
image

After:
image

Copy link
Member

@tomhughes tomhughes left a comment

Choose a reason for hiding this comment

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

I think my main question here is whether we should start these (mostly) expanded or whether they should all be collapsed? Starting them expanded does mean that you're using more vertical space and reducing what top level information can be seen initially?

config/locales/en.yml Outdated Show resolved Hide resolved
@AntonKhorev
Copy link
Collaborator Author

About starting (not) expanded and vertical space:

The first obvious thing that I came up this is to use roughly the same logic other <detail> elements are open/closed by default. Way nodes has this line <details <%= "open" if way.way_nodes.count < 10 %>> which expands way nodes if there are fewer than 10 of them. So I thought I'd expand node ways if there's only one other way for a node. Collapsing just one way won't save much space.

But then I checked some actual ways in osm and looks like their nodes in most of the cases are members of one or two ways. Other cases are relatively rare. If I do what I described in the previous paragraph, I wouldn't have to collapse almost any also part of list. So collapsing 1-way lists doesn't save much vertical space, and collapsing many-way lists also doesn't save much vertical space. What does save it then?

I expect most of space wasted in this case:

  • there's a long way with many nodes
  • nodes mostly don't have any tags
  • parts of the way share nodes with other ways, forming sequences of nodes with the same type (where type is "nothing special", just show the generic node icon), name (actually no name, just show the id) and set of shared ways

Here's an example that has two such sequences with one more special node in between:

image

The idea is then to collect every such sequence into one list element, and that's what #5355 does.

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.

Website element display doesn't know about closed ways ...
2 participants