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: Add a way to display search suggestions #78

Draft
wants to merge 4 commits into
base: custom-list
Choose a base branch
from

Conversation

RemiBardon
Copy link
Member

@RemiBardon RemiBardon commented Jul 11, 2022

It was initially in #76, but I separated the two features.

  • Add preview app in Makefile

@nesium's TODO-list proposition (modified a bit):

1. TCA-driven TextView + Reducer

#119

2. AutoSuggest reducer and environment

  • The environment we have basically
  • The reducer is missing (like the one I sent you in the Gist)
  • The reducer accepts an action that contains a search query, then performs the search via its environment
  • Handles loading, error and empty states

2.1 AutoSuggest list

3. SwiftUI window

  • A custom container view in the same shape as .alert() or .popover()
  • Takes only a View for its content and a Bool Binding to determine if the window is presented or not
  • It would be fantastic if the window could be closed on click outside, but that could require serious hackery (e.g. swizzling NSApp.sendEvent). But maybe there is an easy solution? Misc.: Escape to close sheets #86

4. ContactSuggestionField (or actually just a reducer)

  • It's a data-only component
  • The view is composed from scratch where we need it to handle the case where search results are either displayed inline or in a popover (window)
  • Composes the TCA-driven TextView reducer and AutoSuggest reducer
  • Feeds events into the respective reducer by providing relatively little glue code
  • Manipulates the focus when receiving up/down key events by using the convenience methods in the AutoSuggestList state
    vCloses the window when escape is pressed
  • Inserts token from list selection when return is pressed or an item in the list is clicked
  • Can be hard-coded to contacts for now, we can make it generic when we need it
  • Sets a Bool when it "thinks" that the window with results should be presented/dismissed but its only a hint since the list could be displayed inline

@RemiBardon RemiBardon added the enhancement Improvement request label Jul 11, 2022
@RemiBardon RemiBardon self-assigned this Jul 11, 2022
@RemiBardon RemiBardon marked this pull request as draft July 11, 2022 11:35
@RemiBardon
Copy link
Member Author

Selects the next responder when Tab is pressed

@nesium Why wouldn't this already work? We do not intercept Tab events 🤔

@nesium
Copy link
Contributor

nesium commented Jul 13, 2022

Selects the next responder when Tab is pressed

@nesium Why wouldn't this already work? We do not intercept Tab events 🤔

Is that default NSTextView behavior? Have you tried it?

@RemiBardon
Copy link
Member Author

RemiBardon commented Jul 13, 2022

Is that default NSTextView behavior? Have you tried it?

Yes it is, as long as you check the System Preferences > Keyboard > Shortcuts > "Use keyboard navigation to move focus between controls" checkbox.

I don't know why it's not the default, but it's there.

@RemiBardon RemiBardon changed the base branch from master to conversation-popups July 14, 2022 18:10
@RemiBardon RemiBardon changed the base branch from conversation-popups to master July 14, 2022 18:10
@RemiBardon RemiBardon changed the base branch from master to conversation-popups July 14, 2022 18:11
@RemiBardon RemiBardon changed the base branch from conversation-popups to master July 14, 2022 18:12
@RemiBardon
Copy link
Member Author

There are too many commits cluttering the PR, let me rebase this correctly.

@RemiBardon RemiBardon changed the base branch from master to conversation-popups July 14, 2022 18:27
@RemiBardon RemiBardon force-pushed the search-suggestions branch from d5409e3 to 1df05ce Compare July 14, 2022 18:27
@RemiBardon
Copy link
Member Author

There are still many added files, but I will remove most of them before marking this PR as ready.

@RemiBardon
Copy link
Member Author

RemiBardon commented Jul 20, 2022

Here is a recording of the attached window:

Screen.Recording.2022-07-20.at.11.58.31.mov

What you should see:

  • The List has a background which cannot be removed in plain SwiftUI, which in addition creates square corners.
    • I had to clip the shape, using the window's corners (which I defined)
    • The background color changes between the list state and the empty state
  • Keyboard navigation works, but it's custom listening to , and and across-section index selection
    • It has to be custom because the text field takes the first responder, and I could not work around it (it could surely be done in AppKit, but I didn't try hard).
  • I added a .onTapGesture to list rows, to allow manual selection, but the list has it's own selection, with a larger hit testing shape.
    • I'm not sure I can use the same shape in a flexible manner (I could hard code a content shape, or use Rectangle().scale(1.5), but I'm not a fan of it.
    • I don't think we really care anyway.
  • SwiftUI Lists are always scrollable (until iOS 16 IIRC), which means the List doesn't have a useful intrinsic content size (even using .fixedSize), which in turn means the window height has to be computed by hand…
    • I used the standard row and header sizes, which means that if we introduce rows of a different size, it will start scolling (or be too big)
  • The first list section header is sticky, even though the list has the correct size to be non-scrollable
    • I don't think it's possible to disable this ATM
  • Using a VStack would fix most issues related to Lists, but it would also mean loosing all the native benefits of Lists, and maintaining two layouts (VStack for the attached window but probably keep the List for the non-window suggestions)…
    • We could also have a custom NSTableView / UITableView wrapper with a custom style. From what I see, the Finder search bar and the Xcode autocompletion windows both use either a custom list or a VStack with a style similar to the native lists.

I think that's all I wanted t note 🤔

@RemiBardon RemiBardon mentioned this pull request Jul 26, 2022
4 tasks
@RemiBardon RemiBardon force-pushed the search-suggestions branch from 09aef53 to 9a12d2c Compare July 26, 2022 10:25
@RemiBardon
Copy link
Member Author

This is nice 🙂

Screen.Recording.2022-07-26.at.15.52.43.mov

No more height computation, no more manual clipping, no more background color…

Base automatically changed from conversation-popups to master August 24, 2022 18:16
@RemiBardon RemiBardon mentioned this pull request Aug 24, 2022
10 tasks
@RemiBardon RemiBardon changed the base branch from master to custom-list August 24, 2022 18:41
This reverts commit 70ee5fd.

- feat: WIP Add `TCATextView`
- chore: Add `autoSuggest` higher-order `Reducer`
- feat: Introduce `AutoSuggestList`
- feat: Introduce `.attachedWindow`
- chore: Allow using the attached window corner radius from everywhere
- feat: Use `CustomList` to avoid height computation
@RemiBardon RemiBardon removed their assignment May 15, 2023
@RemiBardon RemiBardon added the stale Not worked on for a long time label May 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement request stale Not worked on for a long time
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants