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

feat(dropdown.tsx): Add originalText prop on Dropdown component #437

Merged
merged 2 commits into from
Oct 18, 2024

Conversation

maryxan
Copy link
Contributor

@maryxan maryxan commented Oct 17, 2024

Description

Add originalText optional prop on Dropdown component.
The purpose of this prop is the ability to have search functionality when the label is JSX Element.
The component will search and return the matches for originalText.

@maryxan maryxan self-assigned this Oct 17, 2024
@@ -2,7 +2,7 @@ import { DropdownItem } from "./types";

export const filterListByKeyword = (list: DropdownItem[], keyword: string): DropdownItem[] => {
Copy link
Collaborator

@xanderantoniadis xanderantoniadis Oct 18, 2024

Choose a reason for hiding this comment

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

Try this:

export const filterListByKeyword = (list: DropdownItem[], keyword: string): DropdownItem[] => {
  const keywordLower = keyword.toLowerCase();

  return list.reduce<DropdownItem[]>((filteredItems, item) => {
    const { items, label, originalText } = item;

    // If the item has sub-items, filter them recursively
    if (items?.length) {
      const filteredSubItems = filterListByKeyword(items, keyword);
      if (filteredSubItems.length) {
        filteredItems.push({ ...item, items: filteredSubItems });
      }
    }

    // Check for keyword match in label or originalText
    const hasLabelMatch = typeof label === "string" && label.toLowerCase().includes(keywordLower);
    const hasOriginalTextMatch = originalText?.toLowerCase().includes(keywordLower);

    if (hasLabelMatch || hasOriginalTextMatch) {
      filteredItems.push(item);
    }

    return filteredItems;
  }, []);
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried the above code, but it seems it does not give me the same results as the original function. See images:

with the above code:
Screenshot 2024-10-18 at 10 55 26 AM

with the original function:
Screenshot 2024-10-18 at 10 55 37 AM

with the above code:
Screenshot 2024-10-18 at 10 56 13 AM

with the original fn:
Screenshot 2024-10-18 at 10 56 21 AM

Copy link
Collaborator

@xanderantoniadis xanderantoniadis Oct 18, 2024

Choose a reason for hiding this comment

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

To me it seems like the original fn doesn't work as expected :/ You search for category and it doesn't return any results even if it has items named "category" :/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure which is the wanted functionality here. The "Category" is not an option on the list but the title of the sub-menu.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we don't have clear specs here. lets leave it as is.

@maryxan maryxan merged commit 7c52136 into main Oct 18, 2024
3 checks passed
@maryxan maryxan deleted the chore/add-originalText-prop-on-dropdown branch October 18, 2024 08:26
github-actions bot pushed a commit that referenced this pull request Oct 18, 2024
# [5.36.0](v5.35.3...v5.36.0) (2024-10-18)

### Features

* **dropdown.tsx:** Add originalText prop on Dropdown component ([#437](#437)) ([7c52136](7c52136))
Copy link

🎉 This PR is included in version 5.36.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

4 participants