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

Add labels to applied static filters #51

Merged
merged 2 commits into from
Nov 4, 2021
Merged

Conversation

nmanu1
Copy link
Contributor

@nmanu1 nmanu1 commented Nov 2, 2021

Allow users to specify group labels for applied static filters. DecoratedAppliedFilters accepts a mapping of fieldId to group label. If no label is provided, it defaults back to using the fieldId as the group label.

J=SLAP-1658
TEST=manual

Check that specifying a group label for a static filter fieldId is reflected when a filter with that fieldId is applied

@coveralls
Copy link

coveralls commented Nov 2, 2021

Coverage Status

Coverage remained the same at 75.269% when pulling 044a6e2 on dev/label-static-filters into 9c5c492 on main.


export interface DecoratedAppliedFiltersConfig {
showFieldNames?: boolean,
hiddenFields?: Array<string>,
labelText?: string,
delimiter?: string,
staticFiltersGroupLabels?: Record<string, StaticFiltersLabelConfig>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be simpler if this was just a mapping of fieldId to label? instead of a mapping of fieldId to an object with a single label property

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that would be simpler. I did it this way to make it more clear what the mapping was between, but I could also just do that with documentation instead

Copy link
Contributor

@oshi97 oshi97 Nov 4, 2021

Choose a reason for hiding this comment

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

Ah ok, let's just go with the string to string mapping for now, that way people don't have to understand what a StaticFiltersLabelConfig is. if needed we can tweak it later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated!

@nmanu1 nmanu1 merged commit 7ea865e into main Nov 4, 2021
@nmanu1 nmanu1 deleted the dev/label-static-filters branch November 30, 2021 00:27
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.

None yet

3 participants