Skip to content

Conversation

@griest024
Copy link
Member

@griest024 griest024 commented Oct 3, 2025

PR Checklist

  • Commit message follows our contributing guidelines
  • Tests added/updated (for bug fixes/features)
  • Documentation added/updated (for bug fixes/features)

PR Type

  • Bug fix
  • Feature
  • Style update
  • Refactor
  • Test
  • Build
  • CI
  • Docs
  • Performance
  • Other (please describe)

Current behavior

Fixes: #3723
Fixes: #3804

New behavior

Breaking change?

  • Yes
  • No

Additional context

@xelaint xelaint added the package: daff.io @daffodil/daff.io label Oct 6, 2025
@xelaint xelaint mentioned this pull request Oct 6, 2025
3 tasks
@xelaint xelaint force-pushed the feat/search/algolia branch from c6e77bc to 8d9504b Compare October 6, 2025 17:10
@xelaint xelaint force-pushed the feat/search/algolia branch from 48b01c7 to b219132 Compare November 5, 2025 19:42
@xelaint
Copy link
Member

xelaint commented Nov 5, 2025

@griest024

  • Retrieve the package that the search result is from and display it under each result
  • No results found div should only be visible if search field has input and no results are found

@griest024
Copy link
Member Author

@xelaint the text color of highlighted items doesn't switch
Screenshot_2025-11-07_16-19-29

@griest024
Copy link
Member Author

@griest024

* [ ]  Retrieve the package that the search result is from and display it under each result

* [ ]  `No results found` div should only be visible if search field has input and no results are found

the package will have to wait for #4178 but I fixed the other issue

@griest024 griest024 force-pushed the feat/search/algolia branch from 64e8bd2 to 2557f26 Compare November 7, 2025 23:24
@griest024
Copy link
Member Author

@xelaint I've added the package to the API search results. It doesn't look great so it will need some styling love from you

@xelaint
Copy link
Member

xelaint commented Nov 10, 2025

Screen Shot 2025-11-10 at 9 07 15 AM

The first five results come from the cart package docs, but it's not really clear. Can the package name also be added to these results?

@xelaint
Copy link
Member

xelaint commented Nov 10, 2025

Screen Shot 2025-11-10 at 9 23 55 AM

Doesn't seem like search history is working. I searched up cart things before this screen. Closed the modal, reopened it, and these are the results that are displayed. When I inspect the code, it seems like these are displayed via the search-results, not search-history.

@griest024
Copy link
Member Author

Screen Shot 2025-11-10 at 9 07 15 AM

The first five results come from the cart package docs, but it's not really clear. Can the package name also be added to these results?

hm should @daffodil/ssr instead show as Overview | @daffodil/ssr then?

@xelaint
Copy link
Member

xelaint commented Nov 10, 2025

Screen Shot 2025-11-10 at 9 07 15 AM The first five results come from the cart package docs, but it's not really clear. Can the package name also be added to these results?

hm should @daffodil/ssr instead show as Overview | @daffodil/ssr then?

Yeah, I think that makes sense?

@griest024
Copy link
Member Author

Screen Shot 2025-11-10 at 9 23 55 AM

Doesn't seem like search history is working. I searched up cart things before this screen. Closed the modal, reopened it, and these are the results that are displayed. When I inspect the code, it seems like these are displayed via the search-results, not search-history.

Its a bit tricky. Searching when typing is an incremental search. Using history from incremental results in this:
Screenshot_2025-11-10_14-33-32
which doesn't seem like what we want. When should a search be added to the history list?

@xelaint
Copy link
Member

xelaint commented Nov 10, 2025

Screen Shot 2025-11-10 at 9 23 55 AM Doesn't seem like search history is working. I searched up cart things before this screen. Closed the modal, reopened it, and these are the results that are displayed. When I inspect the code, it seems like these are displayed via the search-results, not search-history.

Its a bit tricky. Searching when typing is an incremental search. Using history from incremental results in this: Screenshot_2025-11-10_14-33-32 which doesn't seem like what we want. When should a search be added to the history list?

I think search history should only populate for results that have been navigated to?

@xelaint
Copy link
Member

xelaint commented Nov 11, 2025

Accessibility considerations to-do (Elain):

  • ARIA support
  • Check screen reader announcements

@griest024
Copy link
Member Author

@xelaint okay package guides now show the package. search history should be fixed as well.

@xelaint
Copy link
Member

xelaint commented Nov 12, 2025

@griest024 There's still a list of results (that's not part of search hisory) that show up without any input.

Also, when I search "test" and click on the provideDaffSearchResultKindFactories result, I expected provideDaffSearchResultKindFactories to be in the recent results, not the word "test".

Screen Shot 2025-11-12 at 1 48 38 PM

@griest024
Copy link
Member Author

@griest024 There's still a list of results (that's not part of search hisory) that show up without any input.

Also, when I search "test" and click on the provideDaffSearchResultKindFactories result, I expected provideDaffSearchResultKindFactories to be in the recent results, not the word "test".

Screen Shot 2025-11-12 at 1 48 38 PM

Ohh I think I misunderstood. Is there just a "recently clicked on results" list, or is that in addition to the recent queries list?

@xelaint
Copy link
Member

xelaint commented Nov 12, 2025

@griest024 There's still a list of results (that's not part of search hisory) that show up without any input.
Also, when I search "test" and click on the provideDaffSearchResultKindFactories result, I expected provideDaffSearchResultKindFactories to be in the recent results, not the word "test".
Screen Shot 2025-11-12 at 1 48 38 PM

Ohh I think I misunderstood. Is there just a "recently clicked on results" list, or is that in addition to the recent queries list?

There isn't a separate UI for recent queries vs recently clicked on. I think we just want recently clicked, not recent queries.

The screenshot above seems like it's showing the recent queries using the <div class="daffio-docs-search-history"> container, and then the bottom portion (starting from "Testing") is using this block:

@let results = docsResults$ | async;
@let loading = loading$ | async;
@if (results?.length > 0 || loading) {
	<ul class="daffio-docs-search-results" role="listbox"></ul>

This block should only ever be visible if the search field has a value and results related to that value.

Screen Shot 2025-11-12 at 2 20 28 PM

@griest024
Copy link
Member Author

griest024 commented Nov 13, 2025

@xelaint okay I've made updates that I think are what you want. We should probably move the result item to its own component

@xelaint
Copy link
Member

xelaint commented Nov 17, 2025

@griest024 I tried to move the search results and search history to separate components but couldn't get the ActiveDescendantKeyManager to work properly.

Can we also pass in the enter key to store result?

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

Labels

package: daff.io @daffodil/daff.io

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Algolia as a Search Driver Search Modal UI

2 participants