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

Fixes the UI tests (admin_privacy_settings) #6395

Merged
merged 1 commit into from Oct 7, 2014
Merged

Fixes the UI tests (admin_privacy_settings) #6395

merged 1 commit into from Oct 7, 2014

Conversation

mnapoli
Copy link
Contributor

@mnapoli mnapoli commented Oct 7, 2014

There was a notification that wasn't hidden on page load (as it should have been). This is because the JS code that hides it is in jQuery, and is run on the page load. But the message to hide is inside an Angular directive which is being "rendered" on page load.

So I suspect that when jQuery tries to hide the span, it doesn't exist in the DOM as the Angular directive hasn't generated it yet. Later, when we click on the checkbox, jQuery can find the span and hide it correctly.

This fix is not really pretty but messing with AngularJS from jQuery is not really simple :/

Link to the failing test: http://builds-artifacts.piwik.org/ui-tests.master/4951.1/screenshot-diffs/singlediff.html?processed=../processed-ui-screenshots/UIIntegrationTest_admin_privacy_settings.png&expected=UIIntegrationTest_admin_privacy_settings.png&github=UIIntegrationTest_admin_privacy_settings.png

There was a notification that wasn't hidden on page load (as it should have been). This is because the JS code that hides it is in jQuery, and is run on the page load. But the message to hide is inside an Angular directive which is being "rendered" on page load.

So I suspect that when jQuery tries to hide the span, it doesn't exist in the DOM as the Angular directive hasn't generated it yet. Later, when we click on the checkbox, jQuery can find the span and hide it correctly.

This fix is not really pretty but messing with AngularJS from jQuery is not really simple :/
mattab pushed a commit that referenced this pull request Oct 7, 2014
Fixes the UI tests (admin_privacy_settings)
@mattab mattab merged commit 204e6dc into master Oct 7, 2014
@mattab
Copy link
Member

mattab commented Oct 7, 2014

Nice you understood why this was not working. Let's wait for UI tests and see if the UI test now work.

@mnapoli mnapoli deleted the fix-ui-tests branch October 7, 2014 03:44
@mattab mattab added the answered For when a question was asked and we referred to forum or answered it. label Oct 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered For when a question was asked and we referred to forum or answered it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants