freshbooks / ember-responsive

Easy responsive layouts with Ember
https://freshbooks.github.io/ember-responsive/
MIT License
40 stars 19 forks source link

Media classes not injecting during Acceptance Tests #95

Closed tiandavis closed 7 years ago

tiandavis commented 7 years ago

Hi Folks, thanks for putting together ember-responsive. Initially, I had similar expectations as the developer in https://github.com/freshbooks/ember-responsive/issues/93, but it makes sense that the acceptance helper wouldn't change the browser size because the addon itself doesn't change the browser size. But I think I'm running into another issue while analyzing how we can bring the responsive pieces under test.

My issue is the media-* classes don't seem to inject during the acceptance test runs.

image

So even if I pause the acceptance test and manually resize the browser, no responsive changes take place because there are no media-* classes to adjust on.

image

Do we have to do anything with tests/helpers/responsive.js to get the injections working that I missed?

Thanks Everyone!

k-fish commented 7 years ago

Hey @tiandavis

The responsive acceptance helper doesn't set media matches with setBreakpoint, it only mocks the is* properties on the media service

If you want media class names to appear, there are a few ways you can add them, such as modifying the setBreakpoint helper function (in the responsive helper) to set your breakpoint with the matches array, which should update the classNames computed property.

What you are trying to test?

tiandavis commented 7 years ago

Hi @k-fish! 👋

We're on a push to retrofit a mobile responsive UI/UX onto an existing desktop ember application. image

Using ember-responsive, we've made some initial progress so far. image

Injecting the media-* classes helps us avoid using more media queries than we need. image

Our acceptance test suite is mature and it would be awesome if we can run those same tests against a mobile form factor (even if it's just a desktop browser resized). Obviously, this is challenging because web browsers disable window.resizeTo and window.resizeBy APIs. So it is not practical to programmatically resize a web browser during tests.

I'm able to update ./testem.js to resize, at-least, Chrome during an ember test -s run.

module.exports = {
  "framework": "qunit",
  "test_page": "tests/index.html?hidepassed",
  "disable_watching": true,
  "launch_in_ci": [
    "PhantomJS"
  ],
  "launch_in_dev": [
    "PhantomJS",
    "Chrome"
  ],

  "browser_args": {
    "Chrome": [
      "--window-size=320,600"
    ]
  }
};

So my thought is if we can get the media class names to appear during acceptance tests, then we may have a shot at getting the responsive form factor itself under test.

image

I believe we're looking at the relevant piece of ./tests/helpers/responsive.js now:

MediaService.reopen({
  ...

  _forceSetBreakpoint(breakpoint) {
    let found = false;

    const props = {};
    this.get('_breakpointArr').forEach(function(bp) {
      const val = bp === breakpoint;
      if (val) {
        found = true;
      }

      props[`is${classify(bp)}`] = val;
    });

    if (found) {
      this.setProperties(props);
    } else {
      throw new Error(
        `You tried to set the breakpoint to ${breakpoint}, which is not in your app/breakpoint.js file.`
      );
    }
  },

  match() {}, // do not set up listeners in test

  ...

});

...

How should we modify _forceSetBreakpoint to get media class names to appear during acceptance tests?

Thanks a bunch!

k-fish commented 7 years ago

I can see at least two options you can try.

  1. If you are fine with modifying the browser size in your ./testem.js, you can actually get away with not using the breakpoint helper at all, as it only currently allows you to check the change in responsiveness (switching between breakpoints). Remove the response helper import in start-app.js, which will mean the media service doesn't get reopened for every test. Then you'll notice classNames will appear based on your browser size.

  2. If you still want to be able to manipulate the breakpoint for specific tests, you'll have to modify your responsive helper and put a line in to set matches. You'll have to be careful that you reset changes between tests though. Note: the following code is untested 😆

      const val = bp === breakpoint;
      if (val) {
        found = true;
        this.get('matches').push(bp);
      }

Let me know if you need clarification

tiandavis commented 7 years ago

BOOM! Validated both Option 1 and Option 2 gives us something to work with.

image

Thanks for the help here @k-fish !

🎉

k-fish commented 7 years ago

Awesome! Going to close for now. Re-open if you have more questions