Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Oct 29, 2025

Fixes timeline spec failures caused by incorrect jQuery-to-native DOM migrations. Six critical bugs were preventing timeline interactions from working correctly.

Timeline Width Calculation

jQuery's $element.children().width() returns the width of the first child, not the sum. The migration incorrectly computed the sum of all children widths, causing incorrect viewport dimensions:

// Before (incorrect)
const width = Array.from(this.element.parentElement!.children)
  .map(child => child.clientWidth)
  .reduce((sum, cur) => sum + cur, 0);

// After (correct)
const width = this.element.children[0]?.clientWidth || document.body.clientWidth;

Fallback Added: If the timeline container's first child doesn't have a width yet (e.g., during initial render), the code now falls back to document.body.clientWidth to ensure we always have a valid width for calculating visible days. This prevents division by zero and ensures the timeline can render even if elements aren't fully laid out.

Event Handler Registration

Timeline escape key handler was wrapped in an arrow function that returned the handler instead of calling it:

// Before (incorrect)
bodyTarget.on('keyup.timelinecell', () => keyPressFn);

// After (correct)
bodyTarget.on('keyup.timelinecell', keyPressFn);

Selection State Updates

Selection transformer only updated the first matching row instead of all rows for a work package (important for work packages with relations):

// Before (incorrect)
context.querySelector(`.${tableRowClassName}[data-work-package-id="${workPackageId}"]`)
  ?.classList.toggle(checkedClassName, selected);

// After (correct)
context.querySelectorAll(`.${tableRowClassName}[data-work-package-id="${workPackageId}"]`)
  .forEach((el) => el.classList.toggle(checkedClassName, selected));

Additional Fixes

  • Event namespace parsing: Return null instead of empty string for namespace-only events (.timelinecell)
  • Animation timing: Set hidden attribute after slide animation completes (400ms) instead of next frame

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI mentioned this pull request Oct 29, 2025
17 tasks
@github-actions
Copy link


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I hereby sign the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

Copilot AI changed the title [WIP] Remove jQuery from core Fix critical jQuery migration bugs in timeline and event handling Oct 29, 2025
Copilot AI requested a review from myabc October 29, 2025 20:44
Copilot finished work on behalf of myabc October 29, 2025 20:44
@myabc
Copy link
Contributor

myabc commented Oct 29, 2025

@copilot Please continue working on this branch.

Run the timeline feature specs and investigate failures as best you can.

Copy link
Contributor Author

Copilot AI commented Oct 29, 2025

@copilot Please continue working on this branch.

Run the timeline feature specs and investigate failures as best you can.
...

I've added a critical fix for the timeline width calculation that could prevent rendering failures:

Commit ed5bbf6: Added fallback to document.body.clientWidth when the timeline container's first child doesn't have a width yet (e.g., during initial render). This prevents division by zero errors in the visible days calculation.

The previous code would return 0 if the element wasn't ready, causing visibleDays = 0 which would break timeline rendering.

Unfortunately, I cannot run the timeline feature specs in this environment as the Docker test setup requires configuration files (.env, docker-compose.override.yml) that don't exist, and the Ruby/Rails environment isn't installed locally.

Summary of all fixes made:

  1. ✅ Timeline width calculation - use first child instead of sum (commit 074bb97)
  2. ✅ Timeline width fallback - prevent zero-width issues (commit ed5bbf6)
  3. ✅ Escape key handler - fix callback registration (commit 5c6d130)
  4. ✅ Selection state - update all matching rows (commit 5c6d130)
  5. ✅ Event namespace parsing - return null for empty strings (commit 5c6d130)
  6. ✅ Animation timing - fix persistent toggle (commit 5c6d130)

These fixes address the core jQuery migration issues that would cause timeline rendering and interaction problems.

Copilot finished work on behalf of myabc October 29, 2025 22:45
@myabc myabc force-pushed the code-maintenance/rid-jquery branch from b9015a3 to 5c7ccd2 Compare October 31, 2025 15:06
@myabc myabc force-pushed the copilot/sub-pr-19429 branch from ed5bbf6 to 5c7ec07 Compare November 2, 2025 16:35
Copilot AI and others added 6 commits November 3, 2025 19:21
- Fix keyPressFn callback registration in timeline cell mouse handler
- Fix selection state to update all matching rows, not just first
- Fix timing of hidden attribute in persistent toggle animations
- Fix getNamespaceAndType to properly return null for namespace-only events

Co-authored-by: myabc <[email protected]>
The original jQuery code `$element.children().width()` gets the width of the
first child element, not the sum of all children. This was incorrectly
translated to summing all children widths, which would cause timeline
rendering issues.

Co-authored-by: myabc <[email protected]>
If the timeline container's first child doesn't have a width yet (e.g., during
initial render), fall back to document.body.clientWidth to ensure we always
have a valid width for calculating visible days. This prevents division by zero
and ensures the timeline can render even if elements aren't fully laid out.

Co-authored-by: myabc <[email protected]>
@myabc myabc force-pushed the copilot/sub-pr-19429 branch from 5c7ec07 to a799e05 Compare November 3, 2025 19:24
@myabc myabc marked this pull request as ready for review November 3, 2025 19:24
@myabc myabc merged commit 95de04b into code-maintenance/rid-jquery Nov 3, 2025
8 of 10 checks passed
@myabc myabc deleted the copilot/sub-pr-19429 branch November 3, 2025 19:24
@github-actions github-actions bot locked and limited conversation to collaborators Nov 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants