-
-
Notifications
You must be signed in to change notification settings - Fork 151
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
Batched sample registration form with Paste capabilities #2658
base: 2.x
Are you sure you want to change the base?
Conversation
Something that might be confusing with the tabbed view is the "copy" button behavior. It always uses the value of the first column (Sample 1) and copy it over to the other columns. When you view now e.g. "Sample 3" and click this button, one might expect that the current displayed value is copied over to the other columns on the right, but that is not the case. Hence, we might think of either hiding this What do you think? |
If it is not too difficult, I would adapt the functionality to always copy the value displayed in current tab. I think is the less confusing approach, yet the most powerful. Would hide the button when the current tab is not the first otherwise. If you go ahead with adapting the current functionality, when the button is clicked and current tab is not the first, I would also display a confirmation pop-up: The system will copy the value ' |
Thanks for your feedback Jordi! |
Of course I would change the title attribute of the copy button for an updated description. But I guess the new button icon ( |
Ok, implemented the "copy to columns on the right" feature and the PR is ready for review again |
Man this is coool!. Haven't reviewed in detail yet, but I wanted to share a concern about the overlay for the introduction of individual values all-at-once: This component is not specific of field type. User can manually type the text value for each sample. This works great for text-based field, but I think is prone to errors for fields from other types (e.g. select, reference, checkbox, etc.). For instance, if the field is a reference field and the text does not match any value, the system simply flushes the value of the field. And this might be a problem, specially taking into account that form values would usually remain "hidden" under their own tabs. Other examples in wihch this component might lead to confusion are Therefore I think would be better to display the icon for the introduction of individual values all-at-once only for text-based fields. However, I presume that some labs might want this functionality to be available even for fields not fully-compatible with the type (e.g. datetime), maybe because they do a copy&paste from a spreadsheet or similar that comes with the right format from the beginning. So an alternative would be to rely on a senaite registry setting to either render the icon or not for a given sample field. What do you think? |
Yes, this paste option might get tricky with these custom fields. Currently, this code handles it: https://github.com/senaite/senaite.core/pull/2658/files#diff-e3237269d12664a4fc5c68bf03f6cc8b5ef8cd8a82b36168beb3d48ed05792ffR983-R1018 paste_value: (field, value) =>
# handle reference field
controller = @get_widget_controller(field)
if controller
# NOTE: We use here the search API to find the entered text and set it
# only when we find exactly one result
promise = controller.search value
promise.then (data) ->
# only set the value if we get exactly 1 search result
if data["count"] != 1
return
# select the result
items = data["items"]
controller.select(items)
controller.clear_results()
# set all other fields
else if @is_text(field) or @is_textarea(field)
field.value = value
else if @is_radio(field) or @is_checkbox(field)
try
checked = Boolean(JSON.parse(value))
catch
checked = no
field.checked = checked
# date field
else if @is_date_widget(field)
# we need to fetch the date and time input fields
date_input = field.querySelector("input[type='date']")
time_input = field.querySelector("input[type='time']")
try
date = new Date(value)
date_input.value = date.toISOString().split("T")[0]
time_input.value = date.toTimeString().split(" ")[0]
catch error
console.warn error
site.add_notification("Invalid date format", "Please use the format yyyy-mm-dd MM:HH") This means that only exact matches for all reference fields are used, otherwise, the field is skipped. Besides this, Checkbox, Text and Date fields can be (somehow) handled. Maybe you can give it a try with the Patient fields and see what side-effects happen. If it get's too weird, we can maybe disable the button based on a registry setting... |
Description of the issue/feature this PR addresses
This PR enhances the sample add form for better usability and data management
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.