Timelist centering algorithm change and duration formatting change #7194
Conversation
Make duration formatting better
Current Playwright Test Results Summary✅ 15 Passing - Run may still be in progress, this comment will be updated as current testing workflow or job completes... (Last updated on 01/08/2024 11:14:17pm UTC) Run DetailsRunning Workflow e2e-couchdb on Github Actions Commit: c9000cf Started: 01/08/2024 11:12:18pm UTC
|
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Notebook Tests with CouchDB @couchdb Inspect Notebook Entry Network Requests
Retry 1 • Initial Attempt |
2.94% (1)1 / 34 runfailed over last 7 days |
8.82% (3)3 / 34 runsflaked over last 7 days |
Current Playwright Test Results Summary
✅ 175 Passing -
Run may still be in progress, this comment will be updated as current testing workflow or job completes...
(Last updated on 01/08/2024 11:14:17pm UTC)
⚠️ Flakes
📄 functional/plugins/tabs/tabs.e2e.spec.js • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Tabs View Renders tabbed elements
Retry 1 • Initial Attempt |
3.08% (2)2 / 65 runsfailed over last 7 days |
33.85% (22)22 / 65 runsflaked over last 7 days |
📄 functional/plugins/flexibleLayout/flexibleLayout.e2e.spec.js • 1 Flake
Test Case Results
| Test Case | Last 7 days Failures | Last 7 days Flakes |
|---|---|---|
|
Flexible Layout Toolbar Actions @localStorage Add/Remove Container
Retry 1 • Initial Attempt |
0% (0)0 / 59 runsfailed over last 7 days |
1.69% (1)1 / 59 runflaked over last 7 days |
There was a problem hiding this comment.
Everything is good for the duration formatting change, and the list works well as a standalone. The only problem I see is a regression when past events are in view and the user tries to scroll the view: previously, the user was able to scroll the view, leave the thumb at a desired position, and after a timeout the view would scroll back to bring current events into view. Now, the view fights the user for control of the scroll and tries to immediately reset the scroll. This regression should be fixed.
Time.List.7194.scroll.problem.mp4
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #7194 +/- ##
==========================================
- Coverage 55.88% 55.87% -0.02%
==========================================
Files 657 657
Lines 26180 26198 +18
Branches 2546 2548 +2
==========================================
+ Hits 14632 14637 +5
- Misses 10842 10855 +13
Partials 706 706
*This pull request uses carry forward flags. Click here to find out more.
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const START_TIME_COLUMN = 0; | ||
| const END_TIME_COLUMN = 1; | ||
| const TIME_TO_FROM_COLUMN = 2; | ||
| const ACTIVITY_COLUMN = 3; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
| "textColor": "white" | ||
| }, | ||
| { | ||
| "name": "Time since last accident", |
| const examplePlanSmall3 = require('../../../test-data/examplePlans/ExamplePlan_Small3.json'); | ||
|
|
||
| // eslint-disable-next-line no-unused-vars | ||
| const START_TIME_COLUMN = 0; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
| // eslint-disable-next-line no-unused-vars | ||
| const START_TIME_COLUMN = 0; | ||
| // eslint-disable-next-line no-unused-vars | ||
| const END_TIME_COLUMN = 1; |
Check notice
Code scanning / CodeQL
Unused variable, import, function or class
| let currentActivitiesCount = 0; | ||
| let pastActivitiesCount = 0; | ||
| let futureActivitiesCount = 0; | ||
| const styledActivities = activities.map((activity, index) => { | ||
| if (this.timestamp >= activity.start && this.timestamp <= activity.end) { |
There was a problem hiding this comment.
Let's extract this condition to a boolean returning function for readability
Fixed this. Thanks for finding it! |
|
@shefalijoshi to fix failing tests. Thank you! |
| const { getEarliestStartTime } = require('../../../helper/planningUtils'); | ||
| const examplePlanSmall3 = require('../../../test-data/examplePlans/ExamplePlan_Small3.json'); |
There was a problem hiding this comment.
We're all ESM now, so we need to update these imports:
| const { getEarliestStartTime } = require('../../../helper/planningUtils'); | |
| const examplePlanSmall3 = require('../../../test-data/examplePlans/ExamplePlan_Small3.json'); | |
| import { getEarliestStartTime } from '../../../helper/planningUtils.js'; | |
| const examplePlanSmall = JSON.parse( | |
| fs.readFileSync(new URL('../../test-data/examplePlans/ExamplePlan_Small3.json', import.meta.url)) | |
| ); |
| import { expect, test } from '../../../pluginFixtures.js'; | ||
| import examplePlanSmall3 from '../../../test-data/examplePlans/ExamplePlan_Small3.json'; |
There was a problem hiding this comment.
Need to update this to be ESM style and path agnostic (as we do everywhere else)
| import examplePlanSmall3 from '../../../test-data/examplePlans/ExamplePlan_Small3.json'; | |
| const examplePlanSmall3 = JSON.parse( | |
| fs.readFileSync(new URL('../../test-data/examplePlans/ExamplePlan_Small3.json', import.meta.url)) | |
| ); |
Closes #7130 #7161 #7167
Describe your changes:
Change the centering algorithm for timelist activities, prioritizing current and future activities and optimizing (with no scrolling) when there are no past activities.
Make duration formatting better for timelist activities
All Submissions:
Author Checklist
Reviewer Checklist