Skip to content

More render tests #88

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

Merged
merged 1 commit into from
Mar 5, 2016
Merged

More render tests #88

merged 1 commit into from
Mar 5, 2016

Conversation

henrikrudstrom
Copy link
Member

depends on #84
adds tests for anchoring,
AnchorsFill.qml fails, probably because it references a sibling.

@ChALkeR
Copy link
Member

ChALkeR commented Mar 3, 2016

Not sure why the existing tests png images were changed. Is that required. They were smaller (optimized), btw. Could you remove those changes, please?

The actual changes look good, but I would prefer to keep the opacity for now until we decide on that one.

Would it be fine to you if I would rebase this, remove changes present in #84 (those could be discussed separately) and add opacity for now?

@@ -19,4 +19,10 @@ describe('QMLEngine.scope', function() {
expect(parentItem.sum).toBe(6600);
div.remove();
});

it('can reference sibling items by id', function() {
var qml = loader('Parent', this.div);
Copy link
Member

Choose a reason for hiding this comment

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

Does that file even exist?

Copy link
Member Author

Choose a reason for hiding this comment

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

which file?

Copy link
Member

Choose a reason for hiding this comment

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

ScopeParent.qml

Copy link
Member Author

Choose a reason for hiding this comment

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

should be loader(´Sibling´... good your eyes are working at least

@henrikrudstrom henrikrudstrom force-pushed the more-render-tests branch 3 times, most recently from 1ffa6dd to d8847c2 Compare March 4, 2016 14:22
@henrikrudstrom
Copy link
Member Author

cleaned up and rebased, @ChALkeR, @pavelvasev, @Plaristote LGTY?

PR-URL: #88
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@ChALkeR ChALkeR force-pushed the more-render-tests branch from d8847c2 to 1183986 Compare March 5, 2016 16:41
@ChALkeR
Copy link
Member

ChALkeR commented Mar 5, 2016

Rebased against master, optimized images, minor style fixes.
@henrikrudstrom Is everything ok?

@ChALkeR
Copy link
Member

ChALkeR commented Mar 5, 2016

Ok, LGTM. Landing.

@ChALkeR ChALkeR merged commit 1183986 into master Mar 5, 2016
igosoft pushed a commit that referenced this pull request Mar 6, 2016
PR-URL: #88
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
@ChALkeR ChALkeR deleted the more-render-tests branch March 13, 2016 05:35
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.

2 participants