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

Toggle button for ngRestModel #338

Merged
merged 22 commits into from
Aug 8, 2019
Merged

Conversation

boehsermoe
Copy link
Member

Toggle a value between active and inactive. Also can keep a active status unique for a table.

@nadar
Copy link
Member

nadar commented Jul 31, 2019

This is a very good idea, the only draw back against ngrest toggle status is that it contains no scheduling, but an option to make the checkbox unique is very usefull and also something we could use in admin lang ngrest table.

Let me know if its ready for review, don't forget to add a PR :-) thanks

@boehsermoe
Copy link
Member Author

@nadar What do you mean with "scheduling"?

@nadar
Copy link
Member

nadar commented Jul 31, 2019

They NgRest Plugin: "ToggleStatus" does have the option to schedule the time of toggling. (since admin 2.0 with new scheduler system).

https://github.com/luyadev/luya-module-admin/blob/master/src/ngrest/plugins/ToggleStatus.php#L28-L33

@boehsermoe
Copy link
Member Author

For the first there is no more for the theme topic in the admin module. Only this new action button.
The rest is in the cms module.

Copy link
Member

@nadar nadar left a comment

Choose a reason for hiding this comment

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

Please add a unit test and add php doc, everything else looks good.

@nadar
Copy link
Member

nadar commented Aug 8, 2019

Hey @boehsermoe i am looking forward to merge this, i just saw that not all lines are covered by the unit test. not uniqueStatus for example, and setting an active value. Do you think its possible to add those? Thanks a lot

@boehsermoe
Copy link
Member Author

@nadar yes of course.

@codeclimate
Copy link

codeclimate bot commented Aug 8, 2019

Code Climate has analyzed commit b1ed3b8 and detected 14 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 14

The test coverage on the diff in this pull request is 100.0% (50% is the threshold).

This pull request will bring the total coverage in the repository to 22.0% (0.3% change).

View more on Code Climate.

@nadar nadar merged commit 85b53c6 into luyadev:master Aug 8, 2019
@boehsermoe boehsermoe deleted the theme-loader-1916 branch August 8, 2019 19:48
slowfox089 pushed a commit to slowfox089/luya-module-admin that referenced this pull request Dec 10, 2020
* Themes initialize

* Remove unused use

* Translations

* Refactoring: Rename Themes tp ThemeManager

* Travis mysql connection failed

* admin cms settings

* ActiveButton for toggle status

* Moved Theme administration to cmsadmin

* Refactoring

* Changelog

* Refactoring

* Refactoring

* Refactoring

* Update CHANGELOG.md

* Unittest fpr toggle button

* Test custom enable values close luyadev#338

* Unit test for non unique toggle

* More unit test for full coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants