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

Updates to timeslider and demo for Maplibre 3.6.2 #15

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

gregallensworth
Copy link
Collaborator

Refer to OpenHistoricalMap/issues#855

  • index.html demonstration page updated to Maplibre 3.6.2
  • mapstyle.js = replaced various match statements which were giving errors, with == or in statements, and also added literal where arrays were being passed as arguments
  • timeslider.js = adjusted the date filter with an updated one for Maplibre 3.6.2's more strict treatment of expressions, comparisons, and literal

Copy link
Member

@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.

I think this is shippable, though we might want to streamline the expressions a bit to eke out any available performance gains in what’ll end up being a hot code path.

Comment on lines 660 to +662
// warning: we are mutating someone else's map style in-place, and they may not be expecting that
// if they go and apply their own filters later, it could get weird
const osmfilteringclause = [ 'any', ['>', '1', '0'] ];
const osmfilteringclause = ["==", "nodatefilter", ["literal", "nodatefilter"]];
Copy link
Member

Choose a reason for hiding this comment

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

Is the idea that a style designer can opt a layer out of date filtering by wrapping its filter in this expression? If so, perhaps we should instead rely on some special property in the layer’s metadata that we can document.

[
'any',
['!', ['has', 'start_decdate']],
['<=', ['to-number', ['get', 'start_decdate'], -Infinity], ['literal', deccurrent]]
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, OHM vector tiles always set start_decdate and end_decdate to numbers, not strings, so the to-number expression is unnecessary. If we intend this code to apply to third-party tilesets too, then some additional type safety would be reasonable, but we could just as well document a requirement for these fields to be numbers.

[
'any',
['!', ['has', 'end_decdate']],
['>=', ['to-number', ['get', 'end_decdate'], Infinity], ['literal', deccurrent]]
Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, deccurrent is guaranteed to be a number, so there’s no need to wrap it in a literal expression. We only need to wrap arrays and objects in literal expressions.

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.

5 participants