UX: Remove live theme previewing in favor of refresh (PR #9798)

GitHub

The title of this pull request changed from “FEATURE: Remove live theme previewing in favor of refresh” to "UX: Remove live theme previewing in favor of refresh

@eviltrout I’m curious here, what should we be using?

Ember.testing
DiscourseEnv. environment
Discourse.testing

Is there anything which will play better with ember-cli ?

Right now I am trying to figure out what test is hitting this, and see if it would be better to just write location.reload to do nothing :shrug:

Yeah, we should avoid adding any test-specific code to the app. Instead, stub the reload function.

@CvX I cant figure out how to stub location.reload. I tried with sandbox, and the property is unwriteable

Promise rejected during "update some fields": Cannot redefine property: reload

Right. We’ll have to introduce a bit of indirection here: add a method (or a service) that does the location.reload() and stub that method/service.

@CvX Alright a made a little helper called page-reloader which has a method we can stub :+1:

I’m still curious about my initial question though :stuck_out_tongue: because we have so many ways to check for testing

I usually use Ember.testing but I am not sure yet if that breaks in Ember CLI. We should stick with that for now and if it doesn’t work in the future we’ll mass rename.

@eviltrout I really don’t know the best place to put the discourseEnv check to keep the tests from stalling. If I stub the reload in individual tests, the same stub will have to go in every test, and this will come up again in the future where the stub is forgotten. I guess I could stub in the test_helper.js for all JS tests. Or I could keep the check where it is currently in this PR. What is your preference?

@markvanlan does Ember.testing not work?

@eviltrout Maybe it does, but I was trying to avoid using Ember global