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 simple time slider to map view #1

Merged
merged 7 commits into from
Jul 2, 2024

Conversation

DavidMStraub
Copy link

Hi @1ec5, I added a simple time range slider to the map - it doesn't yet do anything, except setting properties on the map view. I can add filtering on the map pins later. How would we now filter the OHM layer?

Copy link
Owner

@1ec5 1ec5 left a comment

Choose a reason for hiding this comment

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

Thanks so much for putting this together – range sliders are very cool and we need to use them more in OHM too. I do wonder if a single-knob slider would be more logical in some of the map views, where we don’t overlay a bunch of pins that you might want to filter. But we can figure that out separately.

src/views/GrampsjsViewMap.js Outdated Show resolved Hide resolved
@DavidMStraub
Copy link
Author

I do wonder if a single-knob slider would be more logical in some of the map views, where we don’t overlay a bunch of pins that you might want to filter.

Yes indeed. Perhaps a single-knob slider and a button/dropdown to select a range of ±5, ±10, ±50, ±100 years around the selected date to filter the pins?

@DavidMStraub
Copy link
Author

DavidMStraub commented Jul 2, 2024

I changed it to a normal slider now and added the filter methods; however it doesn't seem to work yet; or it works only once, to be precise. Not sure what I'm doing wrong.

src/components/GrampsjsMapTimeSlider.js Outdated Show resolved Hide resolved
src/components/GrampsjsMapTimeSlider.js Outdated Show resolved Hide resolved
src/util.js Outdated Show resolved Hide resolved
DavidMStraub and others added 3 commits July 2, 2024 20:42
Co-authored-by: Minh Nguyễn <mxn@1ec5.org>
Co-authored-by: Minh Nguyễn <mxn@1ec5.org>
Co-authored-by: Minh Nguyễn <mxn@1ec5.org>
src/util.js Outdated Show resolved Hide resolved
@1ec5
Copy link
Owner

1ec5 commented Jul 2, 2024

Thanks so much for figuring out a nice-looking UI for this. From what I can tell, the main things to sort out are:

  • Show and hide the time slider based on the active layer
  • How should the time slider filter pins? Would we show all the events up to the selected date? Or should time-based pin filtering be a separate control in the dialog box accessible from the funnel button in the search bar?

Do you want to keep working on this branch or merge into gramps-project#454 and work on it there? (I enabled edits by maintainers so you can push commits directly to that branch.)

Co-authored-by: Minh Nguyễn <mxn@1ec5.org>
@DavidMStraub
Copy link
Author

DavidMStraub commented Jul 2, 2024

Agreed!

Show and hide the time slider based on the active layer

I was thinking we could leave it present all the time if it's also used to filter pins. In the OSM case it would affect only pins, in the OHM case map & pins.

How should the time slider filter pins? Would we show all the events up to the selected date? Or should time-based pin filtering be a separate control in the dialog box accessible from the funnel button in the search bar?

I added the 1924 ± 10 without function but I was thinking the ± x could be clickable to select a wider range. We could then show all the pins in that time period (e.g. in which places did my ancestors have events at in the 18th century).

Do you want to keep working on this branch or merge into gramps-project#454 and work on it there? (I enabled edits by maintainers so you can push commits directly to that branch.)

Yes, let's do it like that, as soon as the pin filtering is implemented we can then merge the main PR.

Edit: I meant yes, let's merge this into your openhistoricalmap-437 feature branch.

@1ec5
Copy link
Owner

1ec5 commented Jul 2, 2024

Sounds good to me!

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.

2 participants