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(select): add search feature for select #759

Merged
merged 10 commits into from
Jan 11, 2024
Merged

Conversation

erbilnas
Copy link
Collaborator

@erbilnas erbilnas commented Dec 25, 2023

Closes #265

I've pushed the commit unverified as I'm facing operating system-related test errors on my local machine. Despite my efforts, I couldn't resolve the issue. I've opened an issue on the relevant project for further investigation.

modernweb-dev/web#2589

@erbilnas erbilnas added enhancement New feature or request bl-select Select Component labels Dec 25, 2023
@erbilnas erbilnas self-assigned this Dec 25, 2023
@buseselvi
Copy link
Contributor

Great job! @erbilnas 🚀 I have a few notes:

  • The search label does not seem centered.
image
  • There should be a divider between search and arrow icons.
image
  • We should see the search icon when there is no search in search, and we should only see loading during the search process. When the results are listed, it should return to the search icon after loading.
image
  • Also, we shouldn't see 3 icons in any case, we can remove the close button when the search icon appears. The close button can be seen again when the search icon disappears (clicking outside). 🙏
image

@erbilnas
Copy link
Collaborator Author

Hey @buseselvi, thanks for your kind feedback!

We should see the search icon when there is no search in search, and we should only see loading during the search process. When the results are listed, it should return to the search icon after loading.

For your feedback on this comment, the loading state isn't for typing but for monitoring the fetch process. For instance, if developers aim to display options through an API call, we need to provide them with the loading state. They can then modify the loading state as needed. We talked about it earlier.

Copy link
Contributor

@fatihhayri fatihhayri left a comment

Choose a reason for hiding this comment

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

Ok.

@buseselvi
Copy link
Contributor

  • can we remove the Select All from the No Data Found result
    image

  • Also, I think while searching Select All works wrong. It should work as select all searched results, not all results that are not currently visible on the screen. @erbilnas

@leventozen leventozen self-requested a review January 5, 2024 08:01
Copy link
Member

@leventozen leventozen left a comment

Choose a reason for hiding this comment

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

I've got some comments but we can take care of them before stable. LGTM! 🚀

src/components/select/bl-select.ts Show resolved Hide resolved
src/components/select/bl-select.ts Show resolved Hide resolved
src/components/select/bl-select.ts Show resolved Hide resolved
@erbilnas erbilnas requested review from ogunb and removed request for Enes5519, ogunb, DamlaDemir and AykutSarac January 8, 2024 13:41
@erbilnas erbilnas requested a review from AykutSarac January 8, 2024 13:41
@leventozen leventozen merged commit 1d5f767 into next Jan 11, 2024
7 checks passed
@leventozen leventozen deleted the feature/265-search-select branch January 11, 2024 11:42
Copy link

🎉 This PR is included in version 2.4.0-beta.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

This was referenced Jan 25, 2024
Copy link

🎉 This PR is included in version 3.0.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
Labels
bl-select Select Component enhancement New feature or request released on @beta released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding search feature to Select Component
5 participants