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.

Sign up for GitHub

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

Jump to bottom

refactor: Migrate Demo Search to use FluentAutocomplete #1599

Merged
merged 15 commits into from
Mar 6, 2024
Merged

refactor: Migrate Demo Search to use FluentAutocomplete #1599

merged 15 commits into from
Mar 6, 2024

Conversation

Hona
Copy link
Contributor

@Hona Hona commented Feb 27, 2024 •
edited
Loading

Pull Request

📖 Description

  • Moved from FluentSearch & a manual FluentMenu -> FluentAutocomplete
  • Added Properties to FluentAutocomplete
    • ShowOverlayOnEmptyResults (default True to be non breaking).
    • ValueText - Allows setting the text inside the input
    • ValueTextChanged - Allows binding to the text changes inside the input
  • Doesn't show the overlay if Items is null or empty

image
Figure: Default state

image
Figure: Keyboard navigable menu

image
Figure: Clears & navigates on enter press OR click

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have added Unit Tests for my new compontent
  • I have modified an existing component
  • I have validate Unit Tests for an existing component

⏭ Next Steps

@Hona Hona marked this pull request as draft February 28, 2024 06:24
dvoituron
dvoituron requested changes Feb 28, 2024
View reviewed changes
Copy link
Collaborator

@dvoituron dvoituron left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I found this new bindable property ValueText very interesting. Thanks.

Could you add new Unit Tests to validate this change?

Could you update your PR description to be more explicit? Adding this new property, adding a screenshot/gif (e.g. this app of the visual result, ...

src/Core/Components/List/FluentAutocomplete.razor.cs Show resolved Hide resolved
src/Core/Components/List/FluentAutocomplete.razor.cs Show resolved Hide resolved
@Hona
Copy link
Contributor Author

Hona commented Feb 29, 2024

I've got some unit tests to write - however the rest should be good to go

@Hona
Copy link
Contributor Author

Hona commented Mar 4, 2024

Sorry - been a bit busy. Going to find some time to write these tests :)

@Hona Hona marked this pull request as ready for review March 5, 2024 22:35
@Hona Hona requested a review from dvoituron March 5, 2024 22:35
@vnbaaij vnbaaij enabled auto-merge (squash) March 6, 2024 12:03
@vnbaaij vnbaaij merged commit 69fe49a into microsoft:dev Mar 6, 2024
4 checks passed
vnbaaij added a commit that referenced this pull request Mar 6, 2024
@Hona @vnbaaij
refactor: Migrate Demo Search to use FluentAutocomplete (#1599)
0f3e67a 
* refactor: Migrate Demo Search to use FluentAutocomplete

* refactor: Code review comments

* fix: Revert breaking change for now

* test: ValueText & ValueTextChanged

* fix: regenerate

* test: Overlay on clear behaviours

* fix: random IDs being generated in test

* fix: valuetext as well

---------

Co-authored-by: Vincent Baaij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dvoituron dvoituron dvoituron approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Hona @dvoituron @vnbaaij