Skip to content

Preserve scroll position on LG->Cohort Members - #9442

Draft
michaelchadwick wants to merge 3 commits into
ilios:masterfrom
michaelchadwick:frontend-7305-preserve-lg-cohort-member-add-scroll-position
Draft

Preserve scroll position on LG->Cohort Members#9442
michaelchadwick wants to merge 3 commits into
ilios:masterfrom
michaelchadwick:frontend-7305-preserve-lg-cohort-member-add-scroll-position

Conversation

@michaelchadwick

@michaelchadwick michaelchadwick commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Fixes ilios/ilios#7305

Lots of time toying with complicated modifiers and scroll event listeners, but finally got a simple one to do the trick that uses the scrollend event so that it only triggers when scrolling stops, not on every scroll event. It only works with modern (Dec 2025 or newer) versions of browsers, but will just be ignored otherwise.

Note: there already exists a PreserveScroll service, but that works when transitioning between routes, and this PR adds an element modifier that can remember scroll position within a route.

@netlify

netlify Bot commented Jul 31, 2026

Copy link
Copy Markdown

Deploy Preview for ilios-frontend ready!

Name Link
🔨 Latest commit 5445c97
🔍 Latest deploy log https://app.netlify.com/projects/ilios-frontend/deploys/6a7a1dfee28c3d00083c074f
😎 Deploy Preview https://deploy-preview-9442--ilios-frontend.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

✅ Visual Diff Report — PASSED

984 images compared: 984 identical

Download the results.

@michaelchadwick
michaelchadwick marked this pull request as ready for review August 3, 2026 17:24
@michaelchadwick

Copy link
Copy Markdown
Contributor Author

NTS: Might be able to fix with different way of tracking the <tbody> vs capturing scroll position.

@michaelchadwick
michaelchadwick marked this pull request as draft August 7, 2026 20:28
@michaelchadwick
michaelchadwick force-pushed the frontend-7305-preserve-lg-cohort-member-add-scroll-position branch from 79e5360 to 250bc52 Compare August 7, 2026 22:10
@jrjohnson

jrjohnson commented Aug 7, 2026

Copy link
Copy Markdown
Member

I think the answer is in packages/frontend/app/components/learner-group/root.gjs

Somewhere in...

  @cached
  get usersForCohortManagerData() {
    // Learnergroup members are only referenced here to trigger a re-computation on membership changes.
    return new TrackedAsyncData(
      this.getUsersToPassToCohortManager(this.args.learnerGroup, this.args.learnerGroup.users),
    );
  }

  get usersForCohortManager() {
    return this.usersForCohortManagerData.isResolved ? this.usersForCohortManagerData.value : [];
  }

  async getUsersToPassToCohortManager(learnerGroup) {
    const cohort = await learnerGroup.cohort;
    const topLevelGroup = await learnerGroup.getTopLevelGroup();
    const currentUsers = await topLevelGroup.getAllDescendantUsers();
    const users = await cohort.users;
    return users.filter((user) => !currentUsers.includes(user));
  }

...there is a place we could cache this data so we didn't return the [] empty during the rebuild.

My suspicion is that it's in:

const topLevelGroup = await learnerGroup.getTopLevelGroup();
    const currentUsers = await topLevelGroup.getAllDescendantUsers();

...but might be somewhere else.

What should happen when a user is moved is that the array decreases by one person and, that will trigger a re-render in the shadow dom, and that one row will get removed.

What is actually happening is that @users.length in cohort-user-manager.gjs is getting set to [] for just a moment. This causes the entire list to get destroyed and recreated.

This is worth tracking down, the scroll position tracking feels like a bandaid to me.

@michaelchadwick
michaelchadwick force-pushed the frontend-7305-preserve-lg-cohort-member-add-scroll-position branch from 250bc52 to 5445c97 Compare August 10, 2026 18:52
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.

Adding cohort members to learner group with + action jumps to top of list, despite any scrolling

2 participants