-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Enhance Widget Layout #4559
Enhance Widget Layout #4559
Conversation
…ts on dashboard page. dimmers.blade.php now runs through an array filled with widgetGroups Voyager.php's 'dimmers()' function now returns an array filled wih WidgetGroups
Removing old, commented code that is not needed anymore.
Updating test and test documentation to fit with the current Dimmer paradigm Adding in a new test to go over a new constraint through this update. That a dimmer group can only have a max three dimmers.
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.
Except Test everything else looks good. 👍
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 think you can remove the first test.
@@ -64,5 +83,7 @@ public function testEachDimmerGroupHasAMaxAmountOfThreeDimmers() | |||
$dimmers = Voyager::dimmers(); | |||
|
|||
$this->assertEquals(2, count($dimmers)); | |||
$this->assertEquals(3, $dimmers[0]->count()); |
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.
This assertion proves what you were trying to prove in the previous test, so I think the other one can be removed.
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.
This test can be removed.
Removing unnecessary test
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.
Looks good, thanks for this PR
Thank you very much for the help as well. It's much appreciated. |
This update contains two changes to two different files.
I had an issue similar to Issue #3185 and was able to implement a solution that works along with the current voyager configuration but allow for more than three widgets to be on the dashboard window.
This is what the dashboard window looks like with 1 - 3 Widgets
This is what the dashboard window looks like with 5 widgets
Please let me know if there is any more information needed