Skip to content

Conversation

peoray
Copy link
Contributor

@peoray peoray commented Sep 5, 2023

Changes proposed ✍️

What

Fixes #1239

🤖 Generated by Copilot at 1cfca44

Added a new memberName filter for members in the frontend UI. Created a new file memberName/config.ts to define the filter config and imported it in filters/main.ts.

🤖 Generated by Copilot at 1cfca44

memberName filter
config file defines options
autumn leaves sorting

Why

How

🤖 Generated by Copilot at 1cfca44

  • Add memberName filter config for filtering members by name (link)
  • Import and use memberName filter config in main.ts (link)

Checklist ✅

  • Label appropriately with Feature, Improvement, or Bug.
  • Add screehshots to the PR description for relevant FE changes
  • New backend functionality has been unit-tested.
  • API documentation has been updated (if necessary) (see docs on API documentation).
  • Quality standards are met.

@joanagmaia joanagmaia self-requested a review September 5, 2023 15:34
@peoray
Copy link
Contributor Author

peoray commented Sep 5, 2023

@joanagmaia this is good to go, hopefully

);
},
apiFilterRenderer(value: StringFilterValue): any[] {
return apiFilterRendererByType[FilterConfigType.STRING]('displayName', value);
Copy link
Contributor

Choose a reason for hiding this comment

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

Also if you notice on string.label.renderer.ts file we do this:
const excludeText = !include ? ' (exclude)' : '';

And since we are not passing any include value, the filter is getting updated like this:
Screenshot 2023-09-06 at 13 56 40

Can you also update the following:

  1. On frontend/src/shared/modules/filters/types/filterTypes/StringFilterConfig.ts add include: boolean to StringFilterValue
  2. Then on frontend/src/shared/modules/filters/components/filterTypes/StringFilter.vue update defaultForm to
const defaultForm: StringFilterValue = {
  value: '',
  include: true,
  operator: FilterStringOperator.LIKE,
};

This won't render the Include Checkbox. It's just a fix to make sure that we don't render (exclude) when we are actually including.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

alright, changes made :)

Copy link
Contributor

@joanagmaia joanagmaia left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
Will try to ship it today 😄

@joanagmaia joanagmaia merged commit 038ed1e into CrowdDotDev:main Sep 6, 2023
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.

"Member name" filter in Members list
2 participants