Custom directive added for authChecking#302
Custom directive added for authChecking#302Kajol-Kumari wants to merge 9 commits intoCloud-CV:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #302 +/- ##
==========================================
+ Coverage 50.62% 50.80% +0.18%
==========================================
Files 66 67 +1
Lines 3771 3785 +14
Branches 444 445 +1
==========================================
+ Hits 1909 1923 +14
+ Misses 1767 1766 -1
- Partials 95 96 +1
Continue to review full report at Codecov.
|
|
@Sanji515 Can you please review this? |
Shekharrajak
left a comment
Shekharrajak
left a comment
There was a problem hiding this comment.
This is looking good and I am expecting similar directives to be implemented.
But think to make it modular: what if we want to have multiple conditions - as input for the directives ?
| import { AuthService } from '../services/auth.service'; | ||
|
|
||
| @Directive({ | ||
| selector: '[appAuthcheck]' |
There was a problem hiding this comment.
better selector name - a generic one
| private viewContainer: ViewContainerRef | ||
| ) { } | ||
|
|
||
| condition: boolean; |
There was a problem hiding this comment.
variable name should define the value it contains.
Try to make a input value for the directive.
Codecov Report
@@ Coverage Diff @@
## master #302 +/- ##
==========================================
+ Coverage 50.16% 50.32% +0.15%
==========================================
Files 66 67 +1
Lines 3873 3887 +14
Branches 450 451 +1
==========================================
+ Hits 1943 1956 +13
Misses 1836 1836
- Partials 94 95 +1
Continue to review full report at Codecov.
|
5 participants
work done in this PR:
All the logic goes in directive and code becomes more cleaner.
It is implementation of one of the idea introduced under GSOC'20 Idea List
GIF showing functionality working fine after doing the changes:
