-
Notifications
You must be signed in to change notification settings - Fork 12
fix: contact book example: Make search great #318
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
base: main
Are you sure you want to change the base?
Changes from 2 commits
74c5d19
990aad8
65c7468
fc64b32
42e351d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,8 +2,8 @@ | |
| // Licensed under the MIT license. | ||
|
|
||
| import { WebUIElement, observable } from '@microsoft/webui-framework'; | ||
| import { Router } from '@microsoft/webui-router'; | ||
| import { api } from '#api'; | ||
| import { Router, isStateful } from '@microsoft/webui-router'; | ||
| import { api, type Contact } from '#api'; | ||
|
|
||
| // Child components used in cb-app.html | ||
| import '#organisms/cb-header/cb-header.js'; | ||
|
|
@@ -21,8 +21,63 @@ export class CbApp extends WebUIElement { | |
| @observable totalFavorites = '0'; | ||
| @observable groups: string[] = []; | ||
|
|
||
| private searchGen = 0; | ||
|
|
||
| private readonly onNavigated = (): void => { | ||
| if (this.searchQuery) void this.applySearch(this.searchQuery); | ||
| }; | ||
|
|
||
| override connectedCallback(): void { | ||
| super.connectedCallback(); | ||
| window.addEventListener('webui:route:navigated', this.onNavigated); | ||
| } | ||
|
|
||
| override disconnectedCallback(): void { | ||
| super.disconnectedCallback(); | ||
| window.removeEventListener('webui:route:navigated', this.onNavigated); | ||
| } | ||
|
|
||
| onSearch(e: SearchChangeEvent): void { | ||
| this.searchQuery = e.detail.value; | ||
| void this.applySearch(e.detail.value); | ||
| } | ||
|
|
||
| private async applySearch(query: string): Promise<void> { | ||
| const gen = ++this.searchGen; | ||
| const root = this.shadowRoot; | ||
| if (!root) return; | ||
|
|
||
| const activeRoute = root.querySelector('webui-route[active]'); | ||
| if (!activeRoute) return; | ||
|
|
||
| const contactsPage = activeRoute.querySelector('cb-page-contacts'); | ||
| const favoritesPage = activeRoute.querySelector('cb-page-favorites'); | ||
| const groupPage = activeRoute.querySelector('cb-page-group'); | ||
|
|
||
| let pageEl: Element | null = null; | ||
| let favorites = false; | ||
| let group = ''; | ||
|
|
||
| if (contactsPage) { | ||
| pageEl = contactsPage; | ||
| } else if (favoritesPage) { | ||
| pageEl = favoritesPage; | ||
| favorites = true; | ||
| } else if (groupPage) { | ||
| pageEl = groupPage; | ||
| group = this.activeGroup; | ||
| } | ||
|
|
||
| if (!pageEl) return; | ||
|
|
||
| const contacts: Contact[] = await api.contacts.list({ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The generation counter prevents stale writes, but it does not cancel in-flight requests, so search-as-you-type can still create one request per keystroke. Route loaders already receive an For example, after threading import type { RouteLoaderContext } from '@microsoft/webui-router';
export class CbPageGroup extends WebUIElement {
@observable contacts: Contact[] = [];
@observable groupName = '';
static async loader({ params, query, signal }: RouteLoaderContext) {
const contacts = await api.contacts.list(
{ q: query.q || undefined, group: params.group },
{ signal },
);
return { contacts, groupName: params.group };
}
}Same pattern can cover contacts and favorites without app-shell DOM probing. |
||
| q: query || undefined, | ||
| favorites: favorites || undefined, | ||
| group: group || undefined, | ||
| }); | ||
|
|
||
| if (gen !== this.searchGen) return; | ||
| if (isStateful(pageEl)) pageEl.setState({ contacts }); | ||
| } | ||
|
|
||
| onSelectContact(e: ContactEvent): void { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| <h2 class="page-title">All Contacts</h2> | ||
| <div class="contact-list-container"> | ||
| <for each="contact in contacts"> | ||
| <cb-contact-card id="{{contact.id}}" first-name="{{contact.firstName}}" last-name="{{contact.lastName}}" | ||
| email="{{contact.email}}" phone="{{contact.phone}}" company="{{contact.company}}" group="{{contact.group}}" | ||
| favorite="{{contact.favorite}}" initials="{{contact.initials}}" avatar-color="{{contact.avatarColor}}" | ||
| notes="{{contact.notes}}" address="{{contact.address}}"></cb-contact-card> | ||
| </for> | ||
| </div> | ||
| <if condition="contacts.length"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This repeats the contact-card loop and empty-state behavior that already exists in <h2 class="page-title">All Contacts</h2>
<cb-contact-list :contacts="{{contacts}}"></cb-contact-list>Then the page TS only needs to import the list component: import '#organisms/cb-contact-list/cb-contact-list.js';If the page-specific container class is needed for styling, let's move/parameterize that in the shared list component rather than duplicating the loop in three pages. |
||
| <div class="contact-list-container"> | ||
| <for each="contact in contacts"> | ||
| <cb-contact-card id="{{contact.id}}" first-name="{{contact.firstName}}" last-name="{{contact.lastName}}" | ||
| email="{{contact.email}}" phone="{{contact.phone}}" company="{{contact.company}}" group="{{contact.group}}" | ||
| favorite="{{contact.favorite}}" initials="{{contact.initials}}" avatar-color="{{contact.avatarColor}}" | ||
| notes="{{contact.notes}}" address="{{contact.address}}"></cb-contact-card> | ||
| </for> | ||
| </div> | ||
| </if> | ||
| <if condition="!contacts.length"> | ||
| <cb-empty-state title="No contacts found" message="Try adjusting your search or add a new contact."></cb-empty-state> | ||
| </if> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| <h2 class="page-title">Favorites</h2> | ||
| <div class="contact-list-container"> | ||
| <for each="contact in contacts"> | ||
| <cb-contact-card id="{{contact.id}}" first-name="{{contact.firstName}}" last-name="{{contact.lastName}}" | ||
| email="{{contact.email}}" phone="{{contact.phone}}" company="{{contact.company}}" group="{{contact.group}}" | ||
| favorite="{{contact.favorite}}" initials="{{contact.initials}}" avatar-color="{{contact.avatarColor}}" | ||
| notes="{{contact.notes}}" address="{{contact.address}}"></cb-contact-card> | ||
| </for> | ||
| </div> | ||
| <if condition="contacts.length"> | ||
| <div class="contact-list-container"> | ||
| <for each="contact in contacts"> | ||
| <cb-contact-card id="{{contact.id}}" first-name="{{contact.firstName}}" last-name="{{contact.lastName}}" | ||
| email="{{contact.email}}" phone="{{contact.phone}}" company="{{contact.company}}" group="{{contact.group}}" | ||
| favorite="{{contact.favorite}}" initials="{{contact.initials}}" avatar-color="{{contact.avatarColor}}" | ||
| notes="{{contact.notes}}" address="{{contact.address}}"></cb-contact-card> | ||
| </for> | ||
| </div> | ||
| </if> | ||
| <if condition="!contacts.length"> | ||
| <cb-empty-state title="No contacts found" message="Try adjusting your search."></cb-empty-state> | ||
| </if> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,14 @@ | ||
| <h2 class="page-title">{{groupName}}</h2> | ||
| <div class="contact-list-container"> | ||
| <for each="contact in contacts"> | ||
| <cb-contact-card id="{{contact.id}}" first-name="{{contact.firstName}}" last-name="{{contact.lastName}}" | ||
| email="{{contact.email}}" phone="{{contact.phone}}" company="{{contact.company}}" group="{{contact.group}}" | ||
| favorite="{{contact.favorite}}" initials="{{contact.initials}}" avatar-color="{{contact.avatarColor}}" | ||
| notes="{{contact.notes}}" address="{{contact.address}}"></cb-contact-card> | ||
| </for> | ||
| </div> | ||
| <if condition="contacts.length"> | ||
| <div class="contact-list-container"> | ||
| <for each="contact in contacts"> | ||
| <cb-contact-card id="{{contact.id}}" first-name="{{contact.firstName}}" last-name="{{contact.lastName}}" | ||
| email="{{contact.email}}" phone="{{contact.phone}}" company="{{contact.company}}" group="{{contact.group}}" | ||
| favorite="{{contact.favorite}}" initials="{{contact.initials}}" avatar-color="{{contact.avatarColor}}" | ||
| notes="{{contact.notes}}" address="{{contact.address}}"></cb-contact-card> | ||
| </for> | ||
| </div> | ||
| </if> | ||
| <if condition="!contacts.length"> | ||
| <cb-empty-state title="No contacts found" message="Try adjusting your search."></cb-empty-state> | ||
| </if> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the main architectural concern:
cb-appnow has to know about concrete route page tags, the router's active DOM shape, API filters, and manualsetState()calls. That bypasses the router/state model we already have.Can we model search as route query state instead and let the router apply route state to the active page? Sketching the direction:
Then the list routes can declare
query="q"and their route state/loader can return filteredcontacts. That keeps search refresh/back/forward/direct-load behavior in the same path as normal WebUI routing.