Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override #qunit-fixture styling, place inside viewport #776

Merged

Conversation

rohitpaulk
Copy link
Contributor

@rohitpaulk rohitpaulk commented Nov 11, 2020

Fixes #764

Screenshot 2020-11-11 at 10 12 29 PM

Tested using this dummy app.

@rohitpaulk rohitpaulk force-pushed the rohitpaulk/fix-qunit-fixture-position branch from dbce66f to a531c8d Compare November 11, 2020 16:39
@rohitpaulk rohitpaulk marked this pull request as ready for review November 11, 2020 16:43
@drewlee
Copy link
Contributor

drewlee commented Dec 3, 2020

Hey @rwjblue and @scalvert, this is something I've also noticed while updating some of our apps. Would it be possible to get this fix merged and released soon so that we don't inadvertently regress the DX? Thanks!

@rwjblue
Copy link
Member

rwjblue commented Dec 4, 2020

Yep, I completely missed when this was posted. Sorry @rohitpaulk, will review today.

@rwjblue rwjblue added the bug label Dec 4, 2020
Copy link
Member

@rwjblue rwjblue left a comment

Choose a reason for hiding this comment

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

As discussed over in the original issue, I think going with the simpler CSS override approach is good enough (even though I generally worry about the coupling, if QUnit decides to change its styling) and the alternative is a bit messy implementation wise (though it would be fully in our control).

@rwjblue rwjblue merged commit 804014f into emberjs:master Dec 4, 2020
@drewlee
Copy link
Contributor

drewlee commented Dec 4, 2020

Thanks @rwjblue and @rohitpaulk!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v5] ember-testing-container is not visible during tests (due to being inside qunit-fixture)
3 participants