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 recent places to search dropdown #902

Merged
merged 4 commits into from
Nov 24, 2024
Merged

Conversation

jaredkhan
Copy link
Collaborator

@jaredkhan jaredkhan commented Nov 16, 2024

Adds a 'recent places' section to the search dropdown.
Only captures items that you have clicked in the search dropdown (not things that you have arrived at via URL or panned to in the viewer)
Has a 'Clear recents' button.
Still works with click and drag to common ancestor etc (didn't know this feature was there!)
I've added it to the homepage too.
Length is limited to 5 items.
Visiting the same item from a search result twice will reposition it in the list rather than duplicating it.
Items are stored in localStorage so that they survive a refresh.

Fixes #878

@davidebbo
Copy link
Collaborator

Works great!

A few comments, all minor:

  • Maybe bump the limit to 8 or 10. Though in my tree, I don't have any 'Popular Places' to compete with. Maybe it can take that into account somehow. i.e. SomeMax - NumOfPopPlaces.
  • Maybe show the taxon name in addition to the vernacular, which is what happens in the regular search result?
  • Maybe add to the list when you actually click on a node and pop up the inline wiki page? I'm not sold on this one, as this could end up spamming the list.

Still works with click and drag to common ancestor etc (didn't know this feature was there!)

Wow, didn't know that either!

@jaredkhan
Copy link
Collaborator Author

Thanks for your comments @davidebbo!
I've bumped the max up to 8.
It now pushes to the recents list when clicking on a node too. See what you think, I think it's fairly sensible but I suppose it depends how many nodes you click on.
It now includes the latin name in the recents listings.

@davidebbo
Copy link
Collaborator

I just tried the changes, and it all feels good in real use, so I'd suggest taking them all. Thanks!

@hyanwong hyanwong merged commit 5b9af7e into OneZoom:main Nov 24, 2024
1 check passed
@hyanwong
Copy link
Member

Great. I'll just merge this then, and we can tweak later if necessary. Thanks so much @jaredkhan

@davidebbo
Copy link
Collaborator

@jaredkhan one possible concern: once we turn that on in the main tree as well, are https://www.onezoom.org/life/ and https://www.onezoom.org/extinct/life/ going to end up sharing the same list? Or can it be smart enough to differentiate them in how they're stored (since they are kept across sessions)?

@jaredkhan
Copy link
Collaborator Author

Yeah good point, it certainly would share the same list at the moment.

@davidebbo
Copy link
Collaborator

davidebbo commented Nov 25, 2024

I think this is worth addressing. Presumably, it's as simple as using a key modifier in localStorage.setItem(), e.g. 'recent_places_extinct' (maybe based on the path segment before the /life segment if any).

@jaredkhan jaredkhan deleted the recent-places branch December 4, 2024 19:29
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.

Fiddly to add the current node to common ancestors
3 participants