-
Notifications
You must be signed in to change notification settings - Fork 3
pos-filter-ui #366
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?
pos-filter-ui #366
Changes from 2 commits
f627fca
d12ba73
a665a97
4c2279c
18e5b85
9544c7a
8e7d4d4
df7b91d
1986394
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 |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export const POS_CURSOR_SESSION_KEY = 'pos-cursor'; | ||
| export const LEADS_CURSOR_SESSION_KEY = 'leads-cursor'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| import { useLocation } from 'react-router'; | ||
| import { PosPath } from '~/modules/types/path/PosPath'; | ||
| import { LEADS_CURSOR_SESSION_KEY, POS_CURSOR_SESSION_KEY } from '~/modules/constants/PosCursorSessionKey'; | ||
|
|
||
|
|
||
| export const useIsPosLeadSessionKey = () => { | ||
| const { pathname } = useLocation(); | ||
| const isLead = new RegExp(`(^|/)${PosPath.Leads}(/|$)`).test(pathname); | ||
| return { | ||
| isLead, | ||
| sessionKey: isLead | ||
| ? LEADS_CURSOR_SESSION_KEY | ||
| : POS_CURSOR_SESSION_KEY, | ||
| }; | ||
| }; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,168 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IconCalendar, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IconCalendarPlus, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IconCalendarTime, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IconCalendarUp, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IconLabel, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IconSearch, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '@tabler/icons-react'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Combobox, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Command, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Filter, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useFilterQueryState, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| useMultiQueryState, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from 'erxes-ui'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { SelectMember, TagsFilter, SelectBrand } from 'ui-modules'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { PosTotalCount } from './PosTotalCount'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { PosHotKeyScope } from '../types/posHotkeyScope'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { useIsPosLeadSessionKey } from '../pos-detail/hooks/UsePosLeadSessionKey'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const PosFilterPopover = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const [queries] = useMultiQueryState<{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| tags: string[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| searchValue: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| created: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updated: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| lastSeen: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| brand: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }>(['tags', 'searchValue', 'created', 'updated', 'lastSeen', 'brand']); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const hasFilters = Object.values(queries || {}).some( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (value) => value !== null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const PosFilterPopover = () => { | |
| const [queries] = useMultiQueryState<{ | |
| tags: string[]; | |
| searchValue: string; | |
| created: string; | |
| updated: string; | |
| lastSeen: string; | |
| brand: string; | |
| }>(['tags', 'searchValue', 'created', 'updated', 'lastSeen', 'brand']); | |
| const hasFilters = Object.values(queries || {}).some( | |
| (value) => value !== null, | |
| ); | |
| const PosFilterPopover = () => { | |
| const [queries] = useMultiQueryState<{ | |
| tags: string[]; | |
| searchValue: string; | |
| created: string; | |
| updated: string; | |
| lastSeen: string; | |
| brand: string; | |
| birthday: string; | |
| }>(['tags', 'searchValue', 'created', 'updated', 'lastSeen', 'brand', 'birthday']); | |
| const hasFilters = Object.values(queries || {}).some( | |
| (value) => value !== null, | |
| ); |
🤖 Prompt for AI Agents
In frontend/plugins/pos_ui/src/modules/pos/PosFilter.tsx around lines 20 to 32,
the 'birthday' query key is missing from the useMultiQueryState keys array so
hasFilters doesn't detect the birthday filter; add 'birthday' to the array of
keys passed to useMultiQueryState (i.e., include 'birthday' alongside 'tags',
'searchValue', 'created', 'updated', 'lastSeen', 'brand') so the birthday filter
state is tracked and hasFilters reflects it.
Outdated
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.
happy birthday
Outdated
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.
🧩 Analysis chain
LGTM! Consider verifying SelectMember filter detection.
The component structure is well-organized and follows coding guidelines correctly. The use of sessionKey from useIsPosLeadSessionKey ensures proper filter state management between POS and Leads views.
One additional consideration: SelectMember.FilterBar (line 160) may have its own query key (e.g., assignedUserId) that might also be missing from PosFilterPopover's hasFilters check. If SelectMember's filter state doesn't trigger the filtered indicator on the Filter.Trigger, you'll need to add its query key to the array in PosFilterPopover as well.
Run the following script to check what query key SelectMember uses:
🏁 Script executed:
#!/bin/bash
# Description: Find the query key used by SelectMember.FilterBar
# Search for the query key definition in SelectMember components
ast-grep --pattern 'useFilterQueryState$$$' frontend/libs/ui-modules/src/modules/team-members/components/SelectMember.tsx
# Also search for any hardcoded query key strings
rg -n "queryKey.*=" frontend/libs/ui-modules/src/modules/team-members/components/SelectMember.tsxLength of output: 60
Add 'assignedTo' to PosFilterPopover.hasFilters
PosFilterPopover currently doesn’t include the SelectMember default queryKey ('assignedTo') in its hasFilters array, so member selections won’t trigger the active-filter indicator. Update hasFilters in PosFilterPopover to include 'assignedTo'.
🤖 Prompt for AI Agents
In frontend/plugins/pos_ui/src/modules/pos/PosFilter.tsx around lines 115 to
167, PosFilterPopover's hasFilters array is missing the SelectMember default
queryKey 'assignedTo', so selecting a member doesn't mark the filter as active;
update the PosFilterPopover component to include 'assignedTo' in its hasFilters
array (add 'assignedTo' alongside the existing keys) so member selections
trigger the active-filter indicator.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { isUndefinedOrNull, Skeleton } from 'erxes-ui'; | ||
| import { useAtomValue } from 'jotai'; | ||
| import { posTotalCountAtom } from '~/modules/states/PosCount'; | ||
|
|
||
|
|
||
| export const PosTotalCount = () => { | ||
| const totalCount = useAtomValue(posTotalCountAtom); | ||
| return ( | ||
| <div className="text-muted-foreground font-medium text-sm whitespace-nowrap h-7 leading-7"> | ||
| {isUndefinedOrNull(totalCount) ? ( | ||
| <Skeleton className="w-20 h-4 inline-block mt-1.5" /> | ||
| ) : ( | ||
| `${totalCount} records found` | ||
| )} | ||
| </div> | ||
| ); | ||
| }; |
|
Collaborator
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. ene ter chigeere ustaagui yu |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| import { atom } from 'jotai'; | ||
|
|
||
| export const posTotalCountAtom = atom<number | null>(null); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| export const PosPath = { | ||
| Index: '/contacts', | ||
| Customers: '/customers', | ||
| Leads: '/leads', | ||
| Companies: '/companies', | ||
| Vendors: '/vendors', | ||
| Clients: '/clients', | ||
| } as const; | ||
| export type PosPath = typeof PosPath[keyof typeof PosPath]; |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,14 @@ | ||||||
| import { IconCashRegister, IconPlus, IconSettings } from '@tabler/icons-react'; | ||||||
| import { Breadcrumb, Button, Separator } from 'erxes-ui'; | ||||||
| import { Breadcrumb, Button, PageSubHeader, Separator } from 'erxes-ui'; | ||||||
| import { PageHeader } from 'ui-modules'; | ||||||
| import { Link, useSearchParams } from 'react-router-dom'; | ||||||
| import { PosRecordTable } from '@/components/PosRecordTable'; | ||||||
| import { useAtom } from 'jotai'; | ||||||
| import { PosCreate } from '@/create-pos/components/index/pos-create'; | ||||||
| import { PosEdit } from '@/pos-detail/components/posDetail'; | ||||||
| import { renderingPosCreateAtom } from '@/create-pos/states/renderingPosCreateAtom'; | ||||||
| import { PosFilter } from '@/pos/PosFilter'; | ||||||
|
||||||
| import { PosFilter } from '@/pos/PosFilter'; | |
| import { PosFilter } from '@/pos/PosFilter'; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/plugins/pos_ui/src/pages/PosIndexPage.tsx
Line: 10:10
Comment:
**style:** Extra trailing spaces after the import statement
```suggestion
import { PosFilter } from '@/pos/PosFilter';
```
How can I resolve this? If you propose a fix, please make it concise.
Outdated
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.
style: Line contains excessive trailing whitespace that should be removed for cleaner code
| <div className="flex flex-col h-full"> | |
| <div className="flex flex-col h-full"> |
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/plugins/pos_ui/src/pages/PosIndexPage.tsx
Line: 22:22
Comment:
**style:** Line contains excessive trailing whitespace that should be removed for cleaner code
```suggestion
<div className="flex flex-col h-full">
```
How can I resolve this? If you propose a fix, please make it concise.
Outdated
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.
style: Missing proper indentation - should align with parent component structure
Prompt To Fix With AI
This is a comment left during a code review.
Path: frontend/plugins/pos_ui/src/pages/PosIndexPage.tsx
Line: 54:54
Comment:
**style:** Missing proper indentation - should align with parent component structure
How can I resolve this? If you propose a fix, please make it concise.| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,6 +7,7 @@ | |
| "dev:apis": "dotenv -e .env -- node scripts/start-api-dev.js", | ||
| "dev:uis": "dotenv -e .env -- node scripts/start-ui-dev.js", | ||
| "create-plugin": "node scripts/create-plugin.js" | ||
|
|
||
|
Collaborator
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. yaahgeed bainaa zaluuu |
||
| }, | ||
| "engines": { | ||
| "pnpm": ">=8", | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify trailing slash handling in API URL usage.
The trailing slash added to
REACT_APP_API_URLcould cause double-slash issues (e.g.,http://localhost:4000//api/endpoint) if the code concatenates paths without accounting for it. Ensure all API calls handle this correctly.Run the following script to check how the API URL is used throughout the codebase:
🏁 Script executed:
Length of output: 55
🏁 Script executed:
Length of output: 16087
Normalize or remove trailing slash in REACT_APP_API_URL
.env.samplecurrently definesREACT_APP_API_URL=http://localhost:4000/, which leads to URLs likehttp://localhost:4000//upload-filewhen concatenated. Either drop the trailing slash in.env.sampleor strip it at runtime (e.g.envApiUrl.replace(/\/$/, '')) before appending paths.🤖 Prompt for AI Agents
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.
ene odoo yajiinaa mi hu
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.
ene odoo yajiinaa mi hu
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.
@KhBaterdene Thank you for confirming! If you've verified that the trailing slash works correctly with your API endpoint handling, that's great. The change looks good to proceed.
Just for future reference, if you notice any double-slash issues in API URLs (like
http://localhost:4000//api/...), you'll know the trailing slash in the.envfile is the place to adjust. ✨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.
@KhBaterdene Glad to hear it's working well now! 🎉
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!