Replaced ember-did-resize-modifier with ember-on-resize-modifier

[st-h]

as discussed in #47, this PR uses ember-on-resize-modifier instead of ember-did-resize-modifier

[ijlee2]

Hi, @st-h. I hope all's well with you.

I want to keep you updated about the status of the issue and your pull request. I was able to fix the failing tests recently. If you'd like, you can rebase your branch to work with the latest CI workflow and npm packages.

I'm continuing to observe if it makes sense to change the underlying resize modifier. I want to ensure that the modifier is well maintained and prepared for upcoming possible changes to Ember, so I haven't made a quick move.

I wanted to clarify that the long wait for a PR review isn't on you. I much appreciate the initial investigation that you made in this PR. 🙂

If it's okay with you, I'd like to leave the PR open but change the status to draft mode for now. Will you let me know what you think?

[st-h]

Hey @ijlee2,

thanks for the update. If you want to merge the PR, I'll be happy to rebase it. As far as the observer goes, I have been using the addon for a few months in production now and haven't had any issues. The observer is fairly easy to swap out (as you can tell from this PR), in case this should be necessary.

For now I'll stay with my fork, in case this won't get merged. Reason is that good performance for the observer is critical in my app and I don't want to add any code, that can already be handled by browsers natively (increases loading times among other things).

[ijlee2]

@st-h I'm glad to hear that the ember-on-resize-modifier has been working well in your production app. I'll definitely use the pull request that you made if the {{on-resize}} modifier ends up replacing {{did-resize}}. Will keep you posted. 👍🏼

[st-h]

Had some trouble with rebasing, but I think it looks good now. Tests run fine locally now. Let's see what CI thinks about it 😁

[ijlee2]

@st-h Thanks. To be safe (with git commit history), can you recreate the code changes, by following these steps?

• Step 1a. Set upstream

  git remote add upstream git@github.com:ijlee2/ember-container-query.git

• Step 1b. Check that upstream was set correctly

  git remote -v
  
  # Output
  upstream  git@github.com:ijlee2/ember-container-query.git (fetch)
  upstream  git@github.com:ijlee2/ember-container-query.git (push)

• Step 2. Get the latest code changes in your main branch

  git checkout main
  git pull upstream main
  yarn install

• Step 3. Reset git history in on-resize branch

  git checkout on-resize
  git reset --hard HEAD~8

• Step 4. Do a rebase (note, origin and upstream are now equal because of Step 2)

  git pull --rebase origin main

• Step 5a. Add a 1st commit, which installs ember-on-resize-modifier (check that the addon is listed in dependencies)

  ember install ember-on-resize-modifier

• Step 5b. Add a 2nd commit, which implements resize check using ember-on-resize-modifier and adds debounce from @ember/runloop (run linting before committing the changes)

• Step 5c. Add a 3rd commit, which uninstalls ember-did-resize-modifier

  yarn remove ember-did-resize-modifier

• Step 6. Force push the commits

  git push -f origin on-resize

[st-h]

@ijlee2 many apologies. I somehow totally missed your comment. If you still want to merge this, I will updated this PR tonight. Would be great if we could merge this, as I don't want to maintain a fork and I honestly have troubles making up time to do so. And thanks a lot for the detailed instructions.

[ijlee2]

@st-h No worries. Yep, if you can rebase your branch, that'd be great!

[st-h]

@ijlee2 I followed your instructions and force pushed the result, however I am somewhat unsure if what we see here is the desired result. I can also create a new branch/PR, if that makes more sense.

Unrelated: Did you had a chance to take a look at removing the css files? #82

[ijlee2]

Thanks for updating the demo app too.

[ijlee2]

I renamed entry to resizeObserverEntry so that people can look up the term to understand the code better.

https://developer.mozilla.org/en-US/docs/Web/API/ResizeObserverEntry/target

I also created a temporary variable called element. I like that its name matches that of the input variable for queryContainer.

[ijlee2]

Thanks again for helping me decide which resize modifier to use. I didn't investigate yet if it's ok to remove the CSS (width and height of 100%). I will do so before making a major release that includes this pull request.

I checked that the demo app works in the following browsers:

Browser	Current version
Chrome	96.0.4664.55
Edge	96.0.1054.43
Firefox	94.0.2
Safari	15.1

🥳

[ijlee2]

As a result of merging this pull request, I think the rendering tests have become flakey in CI. I'll look into what's happening.

An example of a CI run:

# Test compatibility (ember-release, w3, h3)

not ok 15 Chrome 96.0 - [45 ms] - Integration | Component | container-query > When @dataAttributePrefix is updated: The component doesn't update the data attributes
    ---
        actual: >
            Element div#ember279.ember-view.container-query[data-test-container-query][data-cq1-small][data-cq1-tall][data-cq1-ratio-type-a][data-cq1-ratio-type-b][data-cq2-small][data-cq2-tall][data-cq2-ratio-type-a][data-cq2-ratio-type-b] has attribute "data-cq2-small" with value ""
        expected: >
            Element div#ember279.ember-view.container-query[data-test-container-query][data-cq1-small][data-cq1-tall][data-cq1-ratio-type-a][data-cq1-ratio-type-b][data-cq2-small][data-cq2-tall][data-cq2-ratio-type-a][data-cq2-ratio-type-b] does not have attribute "data-cq2-small"
        stack: >
                at DOMAssertions.doesNotHaveAttribute (http://localhost:7365/assets/test-support.js:11146:12)
                at Assert.assert.areDataAttributesCorrect (http://localhost:7365/assets/tests.js:705:42)
                at Object.<anonymous> (http://localhost:7365/assets/tests.js:1275:16)
        message: >
            The container doesn't have the attribute "data-cq2-small".
        negative: >
            false


# Test addon (w3, h3)

not ok 24 Chrome 96.0 - [62 ms] - Integration | Component | container-query > When @features is updated: The component doesn't update the features
    ---
        actual: >
            
        expected: >
            true
        stack: >
                at DOMAssertions.hasText (http://localhost:7365/assets/test-support.js:11446:14)
                at Assert.assert.areFeaturesCorrect (http://localhost:7365/assets/tests.js:663:66)
                at Object.<anonymous> (http://localhost:7365/assets/tests.js:1761:16)
        message: >
            The container meets the feature "small".
        negative: >
            false