Prevent unused slots in the recent preset list.#11810
Prevent unused slots in the recent preset list.#11810tyrasd merged 2 commits intoopenstreetmap:developfrom
Conversation
|
Can you add test so potential regression would be avoided? |
|
Sure, I will do it rn. |
Razen04
force-pushed
the
recent-preset-fix
branch
from
f974217 to
d799bd3
Compare
Razen04
force-pushed
the
recent-preset-fix
branch
from
d799bd3 to
55c4a55
Compare
|
It's really weird that Github gave attribute to some random account, I am trying to fix the commits because it contained my personal name which I don't wish to have. But Github attributed the commits to someone random person, I think I fixed it for now. Anyways I have added the test case, never done before so maybe it is wrong do let me know that. |
matkoniecz
left a comment
matkoniecz
left a comment
There was a problem hiding this comment.
when you select "motorway link" from major roads folder it will not appear in recent presets
yes, it is no longer blocking history slot but it is still a bug
|
That is what's supposed to happen if we are in US or maybe CA too, OSM prevents those from appearing. I think it is intentionally done |
|
no, it is also clearly part of the same bug and unwanted behavior resulting from overcomplex area-specific presets |
|
Ohhh, I thought that was intentional, fine then I will just make it not check the location maybe and bypass that. But then why is the data containing that, as mentioned in the issue by me? |
|
randomly hiding motorway link from recently used entries is definitely not deliberate it is side effect of having special motorway link entry for USA, maybe also other areas (which was well intentioned solution for some other problem but causing trouble due to increased complexity) |
|
Looking at it again: does NOT entirely fix linked issue, but should work for cases where user edited one area then moved to another. So worth merging anyway, I guess? |
| // filtering before slicing to prevent unused slots in the recent preset list, issue #11405 | ||
| recents = _this.recent().matchGeometry(geometry).collection | ||
| .filter(a => !a.locationSetID || validHere[a.locationSetID]) | ||
| .slice(0, 4); |
There was a problem hiding this comment.
we changed how many recent presets are listed from 4 to defined constant and this code also should use it now
There was a problem hiding this comment.
see #9545
@ Razen04 can you please rebase this branch to the latest development branch?
Razen04
force-pushed
the
recent-preset-fix
branch
from
55c4a55 to
804e228
Compare
|
I have updated the code to work with |
|
alternative fix would be to magically replace US-trunk link by regular trunk link and vice versa, but there is no reasonable way to achieve this and may be confusing admittedly, disappearing trunk link is also confusing (this PR fixes not disappearance but failing to use one of recent entries slots) |
3 participants
Fixes #11405
Filtering before slicing prevents the unused slots in the recent preset list, more detail about this in #11405
If we wish to show the Motorway Link and others in the US and CA in the recent list, we can just prevent filtering the recent list based on location, I have already commented in the issue everything here.