-
Notifications
You must be signed in to change notification settings - Fork 106
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
Feature/loader romans #250
Conversation
Codecov Report
@@ Coverage Diff @@
## feature/loader #250 +/- ##
=================================================
Coverage ? 72.89%
Complexity ? 1072
=================================================
Files ? 60
Lines ? 2586
Branches ? 0
=================================================
Hits ? 1885
Misses ? 701
Partials ? 0
Continue to review full report at Codecov.
|
Few fixes: - set() should use callable not Closure - set() now properly process $args and pass them in callable function as parameters - removed few unused properties - few comments
- remove addTabURL from Tabs - more changes in Tabs - some fixes in demos
src/Loader.php
Outdated
@@ -39,8 +39,9 @@ public function init() | |||
parent::init(); | |||
|
|||
if (!$this->shim) { | |||
$this->shim = ['View', 'class' => ['padded segment'], 'style' => ['min-height' => '7em']]; | |||
$this->shim = ['View', 'class' => ['ui active centered inline loader']]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sometimes this shows double-loader. Also if we disable auto-load this shows loading indicator even though it's not loading.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm ok leaving this for now like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rolled back
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work!
Some refactoring, better test, cleanups:@
Added LoaderShim class. Not sure 100% we want it like that though. Will probably change it back.
Added testing of reloadability inside and outside of loader.
Added test for nested loaders
Loader itself now have 'ui segment', which you can change if you wish.
Loader itself is reloaded, not a nested element.
Loader will reload itself unless jsLoad is called()
added ViewTester - really nice component for testing reload-ability.