Skip to content
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

[dataquery/dictionary] Add option to display enums as either value or field in dataquery module #9224

Merged
merged 8 commits into from
May 3, 2024

Conversation

driusan
Copy link
Collaborator

@driusan driusan commented Apr 26, 2024

The dataquery module was showing the backend enum value, rather than the frontend label, which isn't very useful to most users.

This adds an option for the user to select how they want to display enums.

It also updates the menu label from "alpha" to "beta" since the module has now been tested with large data sets.

This adds an option in the data query tool to display Enum data types as
either a field value or the human readable field label.
Copy link
Contributor

@regisoc regisoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 minor comments:

  • Alpha -> Beta changed in the menu bar, but not in the breadcrumbs:
    image
  • switch case for one case only? see comment.
  • Tested on my vm. Label/Value change as expected.

Recording 2024-05-02 at 15 46 48

Comment on lines +76 to +87
switch (props.enumDisplay) {
case EnumDisplayTypes.EnumLabel:
if (props.dictionary.labels && props.dictionary.options) {
for (let i = 0; i < props.dictionary.options.length; i++) {
if (props.dictionary.options[i] == props.value) {
display= props.dictionary.labels[i];
break;
}
}
}
break;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect other cases in the future for enum?
Just wondering why the switch-case with only one option.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I started with both values then realized the EnumValue option didn't actually need any code.. but I think it's also possible people will ask for other cases at some point.

@driusan
Copy link
Collaborator Author

driusan commented May 3, 2024

@regisoc I fixed the breadcrumb and fixed #9221 while I was there.

Copy link
Contributor

@regisoc regisoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

#9221 fixed too.

@driusan driusan merged commit 8b3eed6 into aces:main May 3, 2024
28 checks passed
@ridz1208 ridz1208 added this to the 26.0.0 milestone Jun 6, 2024
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