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

year 0 and ISO 8601 and slider #10

Closed
wants to merge 1 commit into from

Conversation

gregallensworth
Copy link
Collaborator

Refer to OpenHistoricalMap/issues#624

I have made the changes on a branch, but would like them tested before I merge into main, since OHM loads directly from the main branch on Github.

Testing is as follows:

  • check out the code git clone https://github.com/OpenHistoricalMap/leaflet-ohm-timeslider-v2.git
  • switch to the new ISO-8601 branch: git checkout bce-iso8601-compliance
  • start a Python web server: python -m SimpleHTTPServer 9646 or python3 -m http.server 9646
  • point browser at http://localhost:9646/ and try it out

@gregallensworth gregallensworth self-assigned this Oct 30, 2023
Comment on lines +256 to +257
// hack for <0 and >= -1 so we never have a -0.xxx value; without this "0" (1 BCE) exists twice whilst sliding
// this does make a "funny step" in the range input between -0.999999 and -0.000001 but that beats seeing 1 BCE twice
Copy link
Member

Choose a reason for hiding this comment

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

−0.5 is a valid value (around July of 1 BCE). I can get to that date by entering -0000-07-01 into the Change Date dialog but not by dragging the slider.

-0000-07-01

Bringing up the Change Date dialog again formats the date as “7/1/0”, which is not a valid formatted date. It should say either “-0000-07-01” (raw ISO date) or “7/1/-1” (formatted MDY date). This is a single year being represented as three different numbers simultaneously: “-1”, “0”, and “1 (BCE)”.

7/1/0

If we can’t reuse the dropdown menus in the Change Date dialog, I suggest changing it to not format the date and only accept a raw ISO date for consistency with the range control’s year fields.

Copy link
Collaborator Author

@gregallensworth gregallensworth Oct 30, 2023

Choose a reason for hiding this comment

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

−0.5 is a valid value (around July of 1 BCE)

It is valid, but is the same output as +0.5 and both appear in a numeric slider (range input). If I do not have that workaround, then the slider effectively has two copies of 1 BE, the -1 to 0 range and the 0 to 1 range.

I can get to that date by entering -0000-07-01 into the Change Date dialog but not by dragging the slider.

It is working for me. See screengrab below. Can you describe the problem in more detail?

timeslider scroll through 1 BCE

Copy link
Member

@1ec5 1ec5 Oct 30, 2023

Choose a reason for hiding this comment

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

It is valid, but is the same output as +0.5 If I do not have that workaround, then the slider effectively has two copies of 1 BE, the -1 to 0 range and the 0 to 1 range.

I made a typo above yet again. To clarify, this is what I would expect:

ISO 8601 Filter decdate en-US format Explanation
-0001-01-01 −1.0 1/1/2 BCE Beginning of the year 2 BCE
-0001-07-01 −0.5 7/1/2 BCE Middle of the year 2 BCE
-0000-01-01
0000-01-01
0.0 1/1/1 BCE Beginning of the year 1 BCE
0000-07-01 0.5 7/1/1 BCE Middle of the year 1 BCE
0001-01-01 1.0 1/1/1 Beginning of the year 1 CE

It is working for me. See screengrab below. Can you describe the problem in more detail?

jump.mov

@gregallensworth
Copy link
Collaborator Author

I'm going to close this PR and the bce-iso8601-compliance branch, and revisit from a different angle, fixing the decimaldate math. I think Minh is right that the best order of operations is:

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