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

feat(ui): Move checkInTimeline underscan to the left #87096

Conversation

evanpurkhiser
Copy link
Member

Prior to this change, check-in timelines displayed a "underscan" area on the right side of the timeline container. This is the area that remains after we found the optimal rollup + bucket size + minimum underscan size. This area was visually indicated by a hashed gray indicator with a tooltip indicating that this area was "outside" of the selected time range (period).

We received various feedback that the existence of this area is confusing. However, we can't just eliminate this area. You simply cannot divide 60 into 100 (as an example), so we need to constrain the size of the timeline that we render the selected period. We can however, instead of having the underscan at the end, simply place the underscan at the start and mvoe the starting gridline marker to where the actual period starts, and eschew the indication that there is underscan.

There is an important note here. Previously when the underscan was at the end, there was no need to account for the fact that the size of the underscan area will not always evenly fit the bucket sizes. Since the underscan area is the remaining area in the timeline container, there's no way to guarantee that the buckets evenly fit into this area. In the old implementation we simply would not query the last bucket and it would never overflow out of the container.

Now that the underscan is on the left, we need to account for the fact that the buckets may not be aligned to the actual size of the underscan. So we compute an additional underscanStartOffset that we'll use to move the ticks to the left by. This ensures the first bucket starts aligned at the pixel value that represents that bucket start time, allowing all following buckets to be correctly aligned.

Before
clipboard.png

After
clipboard.png

Notice how the underscan has moved to the left side

@evanpurkhiser evanpurkhiser requested review from a team as code owners March 14, 2025 16:59
@evanpurkhiser evanpurkhiser requested review from a team and removed request for a team March 14, 2025 16:59
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 14, 2025
Copy link

codecov bot commented Mar 14, 2025

❌ 5 Tests Failed:

Tests completed Failed Passed Skipped
9943 5 9938 4
View the top 3 failed test(s) by shortest run time
getConfigFromTimeRange divides into days for larger intervals
Stack Traces | 0.003s run time
Error: expect(received).toEqual(expected) // deep equality

- Expected  - 3
+ Received  + 3

@@ -8,17 +8,17 @@
    "intervals": Object {
      "minimumMarkerInterval": 6000,
      "normalMarkerInterval": 7200,
      "referenceMarkerInterval": 6900,
    },
+   "periodStart": 2023-05-15T11:00:00.000Z,
    "rollupConfig": Object {
      "bucketPixels": 0.5,
      "interval": 1800,
      "timelineUnderscanWidth": 56,
      "totalBuckets": 1488,
      "underscanBuckets": 112,
-     "underscanPeriod": 201600,
+     "underscanStartOffset": 0,
    },
-   "showUnderscanHelp": false,
-   "start": 2023-05-15T11:00:00.000Z,
+   "start": 2023-05-13T03:00:00.000Z,
    "timelineWidth": 744,
  }
    at Object.<anonymous> (.../checkInTimeline/utils/getConfigFromTimeRange.spec.tsx:103:20)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
getConfigFromTimeRange Includes dates when the window spans days
Stack Traces | 0.004s run time
Error: expect(received).toEqual(expected) // deep equality

- Expected  - 4
+ Received  + 4

@@ -8,17 +8,17 @@
    "intervals": Object {
      "minimumMarkerInterval": 117.85714285714285,
      "normalMarkerInterval": 120,
      "referenceMarkerInterval": 123.21428571428571,
    },
+   "periodStart": 2023-05-14T20:00:00.000Z,
    "rollupConfig": Object {
      "bucketPixels": 14,
      "interval": 900,
      "timelineUnderscanWidth": 16,
      "totalBuckets": 56,
-     "underscanBuckets": 1,
-     "underscanPeriod": 900,
+     "underscanBuckets": 2,
+     "underscanStartOffset": 12,
    },
-   "showUnderscanHelp": false,
-   "start": 2023-05-14T20:00:00.000Z,
+   "start": 2023-05-14T19:30:00.000Z,
    "timelineWidth": 784,
  }
    at Object.<anonymous> (.../checkInTimeline/utils/getConfigFromTimeRange.spec.tsx:134:20)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)
getConfigFromTimeRange divides into minutes without showing seconds for medium intervals
Stack Traces | 0.004s run time
Error: expect(received).toEqual(expected) // deep equality

- Expected  - 4
+ Received  + 4

@@ -8,17 +8,17 @@
    "intervals": Object {
      "minimumMarkerInterval": 115.38461538461537,
      "normalMarkerInterval": 120,
      "referenceMarkerInterval": 132.69230769230768,
    },
+   "periodStart": 2023-06-15T08:00:00.000Z,
    "rollupConfig": Object {
      "bucketPixels": 13,
      "interval": 900,
      "timelineUnderscanWidth": 20,
      "totalBuckets": 60,
-     "underscanBuckets": 1,
-     "underscanPeriod": 900,
+     "underscanBuckets": 2,
+     "underscanStartOffset": 6,
    },
-   "showUnderscanHelp": false,
-   "start": 2023-06-15T08:00:00.000Z,
+   "start": 2023-06-15T07:30:00.000Z,
    "timelineWidth": 780,
  }
    at Object.<anonymous> (.../checkInTimeline/utils/getConfigFromTimeRange.spec.tsx:72:20)
    at Promise.then.completed (.../jest-circus/build/utils.js:298:28)
    at new Promise (<anonymous>)
    at callAsyncCircusFn (.../jest-circus/build/utils.js:231:10)
    at _callCircusTest (.../jest-circus/build/run.js:316:40)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at _runTest (.../jest-circus/build/run.js:252:3)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:126:9)
    at _runTestsForDescribeBlock (.../jest-circus/build/run.js:121:9)
    at run (.../jest-circus/build/run.js:71:3)
    at runAndTransformResultsToJestFormat (.../build/legacy-code-todo-rewrite/jestAdapterInit.js:122:21)
    at jestAdapter (.../build/legacy-code-todo-rewrite/jestAdapter.js:79:19)
    at runTestInternal (.../jest-runner/build/runTest.js:367:16)
    at runTest (.../jest-runner/build/runTest.js:444:34)
    at Object.worker (.../jest-runner/build/testWorker.js:106:12)

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

Prior to this change, check-in timelines displayed a "underscan" area on
the right side of the timeline container. This is the area that remains
after we found the optimal rollup + bucket size + minimum underscan
size. This area was visually indicated by a hashed gray indicator with a
tooltip indicating that this area was "outside" of the selected time
range (period).

We received various feedback that the existence of this area is
confusing. However, we can't just eliminate this area. You simply
cannot divide 60 into 100 (as an example), so we need to constrain the
size of the timeline that we render the selected period. We can
however, instead of having the underscan at the end, simply place the
underscan at the start and mvoe the starting gridline marker to where
the actual period starts, and eschew the indication that there is
underscan.

There is an important note here. Previously when the underscan was at
the end, there was no need to account for the fact that the size of the
underscan area **will not always evenly fit the bucket sizes**. Since
the underscan area is the remaining area in the timeline container,
there's no way to guarantee that the buckets evenly fit into this area.
In the old implementation we simply would not query the last bucket and
it would never overflow out of the container.

Now that the underscan is on the left, we need to account for the fact
that the buckets may not be aligned to the actual size of the underscan.
So we compute an additional underscanStartOffset that we'll use to move
the ticks to the left by. This ensures the first bucket starts aligned
at the pixel value that represents that bucket start time, allowing all
following buckets to be correctly aligned.

Before
<img alt="clipboard.png" width="968" src="https://i.imgur.com/nirhIrF.png" />

After
<img alt="clipboard.png" width="970" src="https://i.imgur.com/uOVIdLJ.png" />

Notice how the underscan has moved to the left side
@evanpurkhiser evanpurkhiser force-pushed the evanpurkhiser/feat-ui-move-checkintimeline-underscan-to-the-left branch from f8e2370 to 6a0b6c3 Compare March 14, 2025 18:37
@evanpurkhiser evanpurkhiser merged commit 3bb792d into master Mar 14, 2025
41 checks passed
@evanpurkhiser evanpurkhiser deleted the evanpurkhiser/feat-ui-move-checkintimeline-underscan-to-the-left branch March 14, 2025 19:59
@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants