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 #1310: Travis issue fixed #1316

Merged
merged 1 commit into from
Dec 19, 2018
Merged

Fixes #1310: Travis issue fixed #1316

merged 1 commit into from
Dec 19, 2018

Conversation

Pipe-Runner
Copy link
Contributor

@Pipe-Runner Pipe-Runner commented Dec 19, 2018

Fixes #1310

Checklist

  • I have read the Contribution & Best practices Guide and my PR follows them.
  • My branch is up-to-date with the Upstream master branch.
  • I have added necessary documentation (if appropriate)
  • Added Surge preview link

Changes proposed in this pull request:

  • I have reused hidden instead of ngIf

@codecov
Copy link

codecov bot commented Dec 19, 2018

Codecov Report

❗ No coverage uploaded for pull request base (development@59481bb). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             development   #1316   +/-   ##
=============================================
  Coverage               ?   54.9%           
=============================================
  Files                  ?      51           
  Lines                  ?    1417           
  Branches               ?     175           
=============================================
  Hits                   ?     778           
  Misses                 ?     534           
  Partials               ?     105

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 59481bb...10d5904. Read the comment docs.

@subhahu123
Copy link
Contributor

what is ngif

@Pipe-Runner
Copy link
Contributor Author

what is ngif

@subhahu123 ?? Angular directive. Sorry I didn't get the context of this question. Was it something else you wanted to ask?

@subhahu123
Copy link
Contributor

subhahu123 commented Dec 19, 2018

i have seen your changes but ended up with nothing (i don't know much about angular)
what was problem in travis cl and what have u done to fix .
just an overview
@AakashMallik

@Pipe-Runner
Copy link
Contributor Author

i have seen your changes but ended up with nothing (i don't know much about angular)
what was problem in travis cl and what have u done to fix .
just an overview
@AakashMallik

It was caused because [hidden] just sets the display to 0 while ngIf removes the node completely. The tests check whether or not those nodes are available in the DOM tree. So, when ngIf was used, it was unable to find that node. That is why travis was failing.

Copy link
Member

@rajvaibhavdubey rajvaibhavdubey left a comment

Choose a reason for hiding this comment

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

Changes LGTM 👍 , though the indentation is not uniform throughout the file.

Copy link
Member

@shreyanshdwivedi shreyanshdwivedi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@praveenojha33 praveenojha33 merged commit 7efa87c into fossasia:development Dec 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants