Skip to content

Conversation

@labkey-martyp
Copy link

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

  • Add tpl to dropdown

@bbimber
Copy link
Collaborator

bbimber commented May 27, 2025

@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.

@bbimber
Copy link
Collaborator

bbimber commented May 27, 2025

thanks!

@bbimber
Copy link
Collaborator

bbimber commented May 27, 2025

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?

https://github.com/LabKey/platform/blob/309189f08e712deeca426d744f3dbff30eae9e39/core/webapp/extWidgets/LabkeyCombo.js#L39

@labkey-martyp
Copy link
Author

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.

@labkey-martyp
Copy link
Author

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?

https://github.com/LabKey/platform/blob/309189f08e712deeca426d744f3dbff30eae9e39/core/webapp/extWidgets/LabkeyCombo.js#L39

I'll check it out

@bbimber
Copy link
Collaborator

bbimber commented May 27, 2025

@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.

@labkey-martyp
Copy link
Author

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?

https://github.com/LabKey/platform/blob/309189f08e712deeca426d744f3dbff30eae9e39/core/webapp/extWidgets/LabkeyCombo.js#L39

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants