-
-
Notifications
You must be signed in to change notification settings - Fork 144
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
Allow manual selection of units on results entry #2201
Conversation
+ A list of units can be defined for an analysis service + The user can select the units when submitting data.
update the code so that the default value is always listed first. update the code so that the units rendered beside the result do not wrap.
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.
@@ -1065,7 +1112,18 @@ def _folder_item_method(self, analysis_brain, item): | |||
item["Method"] = api.get_title(method) | |||
item["replace"]["Method"] = get_link_for(method, tabindex="-1") | |||
|
|||
def _on_unit_choice_change(self, uid=None, value=None, item=None, **kw): |
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.
You don't need this if you do it differently. Hint: You don't need a "UnitChoices" column, but a "Unit" column that becomes editable (with a choices list) when the analysis have multiple units set. Otherwise, the column "Unit" is not rendered.
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.
Removed UnitChoice column. Unit it column is only rendered when needed.
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 this function _on_unit_choice_change
, cause on_unit_change
is used instead
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.
Thank you, removed.
Implemented corrections from senaite#2201 (review)
Thank you @xispa for all your comments. I have implemented your comments (ac745f9). A list of units for selection can be provided here: The unit column will only render if a selection of units is provided. I have also updated the code so that the units render after the uncertainty. The units change is now stored properly: Improvement: |
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.
Thanks, you are almost there!. Just two minor changes (see comments) and please add an entry at CHANGES.rst. Also, do a pull first, cause I've merged 2.x into your branch and done some commits to make Flake8 happy.
@@ -1065,7 +1112,18 @@ def _folder_item_method(self, analysis_brain, item): | |||
item["Method"] = api.get_title(method) | |||
item["replace"]["Method"] = get_link_for(method, tabindex="-1") | |||
|
|||
def _on_unit_choice_change(self, uid=None, value=None, item=None, **kw): |
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 this function _on_unit_choice_change
, cause on_unit_change
is used instead
Implemented xispa requested changes on Jan 19. see senaite#2201
Hi @xispa, |
Description of the issue/feature this PR addresses
Linked issue: https://github.com/senaite/senaite.core/issues/
Current behavior before PR
Desired behavior after PR is merged
--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.