-
Notifications
You must be signed in to change notification settings - Fork 5
refactor(soup): refactor soup and unified list implementation #1248
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?
Conversation
…on filter or sort change
…t and replace query with generic source. Add EntityRow stuff
| page_size?: number; | ||
| } | ||
|
|
||
| interface EntitiesQueryRequesdtBody { |
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.
typo
| // ============================================================================ | ||
| // Entity Type Filters | ||
| // ============================================================================ |
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.
remove claudonian comment
|
|
||
| export const SoupViewContext = createContext<SoupViewContextValues>(); | ||
|
|
||
| export const useSoupView = () => { | ||
| const context = useContext(SoupViewContext); | ||
|
|
||
| if (!context) { | ||
| throw new Error( | ||
| 'useSoupView can only be used under a SoupViewContext.Provider' | ||
| ); | ||
| } | ||
|
|
||
| return context; | ||
| }; | ||
|
|
||
| export const useMaybeSoupView = () => useContext(SoupViewContext); |
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.
maybe just use the createContextProvider from solid primitives ? or the asserted one ?
| if (types.length === 0) types = []; | ||
| const includeArray: UnifiedSearchIndex[] = []; | ||
| for (const type of types) { | ||
| switch (type) { |
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.
nit: uise ts-pattern's match
| // ============================================================================ | ||
| // Lazy Initialization (avoids creating signals at module load time) | ||
| // ============================================================================ | ||
|
|
||
| let _priorityLabelToggles: SignalToggle<string>[] | null = null; | ||
| let _priorityMetadataToggles: SignalToggle<EmailMetadataKey>[] | null = null; | ||
| let _depriorityLabelToggles: SignalToggle<string>[] | null = null; | ||
| let _depriorityMetadataToggles: SignalToggle<EmailMetadataKey>[] | null = null; | ||
|
|
||
| let _priorityLabels: Accessor<Set<string>> | null = null; | ||
| let _depriorityLabels: Accessor<Set<string>> | null = null; | ||
| let _priorityMetadata: Accessor<Set<EmailMetadataKey>> | null = null; | ||
| let _depriorityMetadata: Accessor<Set<EmailMetadataKey>> | null = null; |
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 seems completely unnecessary.
| readonly id: string; | ||
| readonly label: string; | ||
| readonly filterIds: readonly string[]; | ||
| readonly allowMultiple?: boolean; |
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.
I don't think allowMultiple is used. Would be nice to configure groups to not be mutually exclusive if we wanted to.
| label: 'Mail', | ||
| predicate: emailFilter, | ||
| group: 'type', | ||
| shortcut: 'l', |
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.
Do we need the shortcuts here? Can't we let the hotkey logic handle deciding what filter gets bound to what key ?
…-unified-list-and-soup-implementation
No description provided.