Remove CourseRollover render modifiers

[michaelchadwick]

Refs ilios/ilios#5374

I could not find a way to get a reference to the root DOM element of a component, which is needed for some loading, so I made a new custom modifier, get-element, which feels like a side-step from {did-insert} instead of a true replacement, but is also what Ember recommends creating, so...all good? :D

[jrjohnson]

Great idea to sub in out own modifier where it is very much needed here. Some other cleanup needed as well though before this is ready.

[jrjohnson]

This usage doesn't track for me though. There isn't anything here that needs access to the DOM. This data should be loaded in a TrackedAsyncData and processed in a getter.

[michaelchadwick]

The reason for adding that there is that the Course::RolloverDatePicker is waiting for the Year <select> element to load before it gets the selectedYear + 1 value. If I don't have that there, then the Year <select> loads, say, 2025, and the date picker remains on 11/26/2024 instead of rolling over to 11/25/2025 like it currently does on demo.

[jrjohnson]

How about wrapping everything in an if so it only tries to render once the data needed is resolved?

[michaelchadwick]

Is not the whole thing already wrapped in an if?

{{#if this.allCoursesData.isResolved}}
  <select
    id="year-{{templateId}}"
    data-test-year
    {{on "change" this.setSelectedYear}}
    {{get-element this.loadValidYear}}
  >
    {{#each this.years as |year|}}
      ...
    {{/each}}
  </select>
{{else}}
  <LoadingSpinner />
{{/if}}

[jrjohnson]

If this.allCourses what? It's working off of the null and that's valid, but it's hard to read it here. Would prefer if this.allCoursesData.isResolved.

[michaelchadwick]

Changing it to {{#if this.allCoursesData.isResolved}} just makes the DatePicker not show up at all. If I change it to {{#if (is-array this.allCourses)}}, like the usage for the other <select> that holds this.years, then it works. It seems they both check to make sure it's not null.

[michaelchadwick]

After changing the getters, this is now serviced by the {{#if this.allCoursesData.isResolved}}.

[jrjohnson]

Let's be harsher here. Grab import { assert } from '@ember/debug'; and use it to ensure the callback is a function. That will throw nicely in development mode.

[michaelchadwick]

I tried this approach, but couldn't get the new real test to pass for when the modifier is given an invalid/missing callback. I get a global error from qunit on the render(hbs``) call, and no mangling of the global window error event listeners seemed to quell it. There's no example in the codebase where we do this, so I'm currently out of ideas.

[jrjohnson]

I haven't done this one either, fun new stuff! Looks like maybe you can use assert.Throws from qunit.

Possibly like:

assert.throws(await render(hbs`<div id='root-element' {{get-element}}></div>`));

If that doesn't work I'd be ok with skipping the second test.

[michaelchadwick]

I tried my hardest, but all I get in the end is a global failure in qunit due to the throwing of an Error. I put in a todo for now.