Skip to content
This repository has been archived by the owner on Dec 10, 2024. It is now read-only.

Fix testing root element styles not working #43

Merged
merged 2 commits into from Apr 4, 2015
Merged

Fix testing root element styles not working #43

merged 2 commits into from Apr 4, 2015

Conversation

ghost
Copy link

@ghost ghost commented Mar 4, 2015

I noticed that the testing element wasn't styled properly in the browser. Looks like the CSS isn't added if no options are configured for the addon. Additionally it looks like the positioning of the element needs to be fixed rather than absolute like with qunit.

Christopher Chow added 2 commits March 4, 2015 12:21
The CSS provided by mocha differs from qunit's so absolute
positioning doesn't work.
If no options are provided for this addon, the additional CSS
isn't appended to test-support CSS file. Default options to empty object so
the conditional passes.
@@ -42,7 +42,7 @@ module.exports = {
'vendor/ember-cli-mocha/test-loader.js'
];

var addonOptions = app.options['ember-cli-mocha'];
var addonOptions = app.options['ember-cli-mocha'] || {};
if (addonOptions && !addonOptions.disableContainerStyles) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why the initialization to {} is needed. Won't the first check for the truthiness of addonOptions cover the use case in which the option doesn't exist?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch my comment above - I re-read the conditional and see that we need this by default. Sorry!

dgeb added a commit that referenced this pull request Apr 4, 2015
Fix testing root element styles not working
@dgeb dgeb merged commit ac24d37 into ember-cli:master Apr 4, 2015
@dgeb
Copy link
Member

dgeb commented Apr 4, 2015

Thanks @Soliah!

@Turbo87 Turbo87 added the bug label Dec 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants