-
Notifications
You must be signed in to change notification settings - Fork 1
Add brackets in UserEditableCombo dropdowns around broken lookups #247
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
Conversation
@labkey-martyp: can you please make this an opt-in feature with a config option like 'useBracketsForUnknownValues: false" or something to that effect, and turn this on the in ~2 places in EHR that rely on this feature? There are valid use-cases where users are expected to add values and this is going to change those. Note: the LabKey query XML already supports a config option for lookups called "useRawValue". That was added a while after UserEditableCombo was created. One intent of this was to allow a sort of "soft" lookup, where the field has a lookup configured, but when rendering the raw value is used (i.e., no brackets for broken lookups). A more invasive but logical extension of what you're doing here would be to read this property in the client-side code that translates from the query metadata into Ext4 field config. |
thanks! |
hi @labkey-martyp: I believe EHR frequently uses LABKEY.ext4.ComboBox, right? Or does it inherit through LDK? If it's using the former, have you considered how this tpl override would interact with this? |
Ok yeah the brackets should be opt-in now. I'm not familiar with the soft link version of useRawValue. I've only seen it in the context of using the fk value instead of default display column. It was my understanding it still indicates a broken lookup if the value was not in the fk table. |
I'll check it out |
@labkey-martyp: I cant speak to every center, but the fact that UserEditableCombo exists implies there are cases where there is a lookup defined, but that it's acceptable for users to enter another value. That is what i'm calling a "soft" lookup here. Like you point out, useRawValue=true causes grids to render without brackets when there are broken lookups, which is highly similar to what is happening on the client here. I dont think this is an important enough different to trigger a lot of new development here, but if the relevant EHR/LDK code ever gets touched, reading and interpreting useRawValue=true would make some sense. |
Yeah you're right. The innerTpl is getting overwritten and it's virtually impossible between the core LK extjs component and EHR customizations how to handle the innerTpl. |
Rationale
UserEditableCombo is now the default combo input if there are broken lookups. This adds a tpl to put the broken lookups in the dropdown in brackets.
Related Pull Requests
Changes