[Multicheck control] #4529
[Multicheck control] #4529
Conversation
Yes, please. |
| this.setup_select_all(); | ||
| } | ||
| }, | ||
|
|
There was a problem hiding this comment.
will be good to have helper methods get_checked and get_unchecked
also implement change event
|
| }, | ||
|
|
||
| get_checked_options() { | ||
| return JSON.parse(this.selected_options); |
There was a problem hiding this comment.
why store in JSON and not native objects?
| self.save() | ||
|
|
||
| def on_update(self): | ||
| for d in self.active_domains: | ||
| domain = frappe.get_doc('Domain', d.domain) | ||
| for d in json.loads(self.active_domains): |
There was a problem hiding this comment.
this should be handled in client side. server has nothing to do with controls!
There was a problem hiding this comment.
So maybe we could parse Multi Check control values as list by default ?
There was a problem hiding this comment.
Yes, had tried to set their value as a list itself, but wasn't sure how we'd store the field value in the database then.
| @@ -41,6 +41,14 @@ frappe.utils = { | |||
| is_md: function() { | |||
| return $(document).width() < 1199 && $(document).width() >= 991; | |||
| }, | |||
| is_json_string: function(str) { | |||
|
|
||
| get_select_buttons() { | ||
| return $(`<div><button class="btn btn-xs btn-default select-all" | ||
| style="margin-right: 5px;">${__("Select All")}</button> |
There was a problem hiding this comment.
styling in controls.less ?
|
|
||
| get_checkbox_element(option) { | ||
| return $(` | ||
| <div class="checkbox unit-checkbox" style="color: #36414c;"> |
There was a problem hiding this comment.
styling in controls.less ?
| if(frm.domain_editor) { | ||
| frm.domain_editor.set_items_in_table(); | ||
| } | ||
| frm.fields_dict.active_domains.get_data = () => { |
There was a problem hiding this comment.
The standard pattern is to use frm.set_df_property(fieldname, 'options', options_array)
| self.save() | ||
|
|
||
| def on_update(self): | ||
| for d in self.active_domains: | ||
| domain = frappe.get_doc('Domain', d.domain) | ||
| for d in json.loads(self.active_domains): |
There was a problem hiding this comment.
So maybe we could parse Multi Check control values as list by default ?
| // rename this file from _test_[name] to test_[name] to activate | ||
| // and remove above this line | ||
|
|
||
| QUnit.test("test: Domain Settings", function (assert) { |
| # -*- coding: utf-8 -*- | ||
| # Copyright (c) 2017, Frappe Technologies and Contributors | ||
| # See license.txt | ||
| from __future__ import unicode_literals |
|
Should not be a database field.
Sent from phone
… On 30-Nov-2017, at 8:26 AM, Prateeksha Singh ***@***.***> wrote:
@pratu16x7 commented on this pull request.
In frappe/core/doctype/domain_settings/domain_settings.py:
> self.save()
def on_update(self):
- for d in self.active_domains:
- domain = frappe.get_doc('Domain', d.domain)
+ for d in json.loads(self.active_domains):
Yes, had tried to set their value as a list itself, but wasn't sure how we'd store the field value in the database then.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
Update (27-11-2017)
Has Domainchild table, and consequently the doctype.get_checked_values()andget_unchecked_values(), thoughget_checked_values()is semantically the parsed value of the control itself.Will affect: frappe/erpnext#11709
UI: multiple checkboxes
Value: Comma-separated string of values
Options: Array of label/value option objects
Came across three different use cases, replaced.
Name bad, but
multiselectalready exists.No doctype option even though most of time it would fetch a document name list, to decouple the getting data operation. Handles async fetches.
select_alloptionalCheck box editor removed, functionality covered except the one of mapping values to child tables, instead implemented when needed, as in Domain Settings (important enough to be abstracted as before?)
Comma-separation followed from the
multiselectcontrol, now I think a parsable JSON array should be used.Value updates:
