feat: add function for number of rows filtered #56
Conversation
Signed-off-by: Yash Khare <yash2010118@akgec.ac.in>
|
Ready for review @ramonverse |
src/components/FilterBlock.tsx
Outdated
| @@ -60,6 +61,7 @@ const Filter: React.FC<FilterProps> = ({ | |||
| query, | |||
| }) => { | |||
| const [fieldValues, setFieldValues] = useState<string[]>([]) | |||
| const [val,setVal] = useState(0) | |||
There was a problem hiding this comment.
change to rowsFiltered, setRowsFiltered
src/components/FilterBlock.tsx
Outdated
| setFieldValue={(value) => onFilterChange({ ...filter, value })} | ||
| setFieldValue={(value) => { | ||
| onFilterChange({ ...filter, value }) | ||
| filter.value=value; |
There was a problem hiding this comment.
don't assign values to a property
There was a problem hiding this comment.
Forgot to remove it when I had been testing. Will remove that!
src/lib/utils.ts
Outdated
| // SQL | ||
| // return `select * from table where (${filter}) (${operator}) (${value})` | ||
| // PRQL | ||
| if(filter && operator){ |
There was a problem hiding this comment.
all these logic is duplicated here and in filterblock. You could create a utils function like this to avoid bugs and duplications:
export const getPrqlFilter = (operator: string): string => { ... }
There was a problem hiding this comment.
If I am getting this correct, is it like this
export const getPrqlFilter = (operator:string):string =>{
if(operator=='equals'){
return `==`
}
if(operator=='notEquals'){
return `!=`
}
if(operator=='contains'){
return `std.text.contains this.`
}
return ""
}
|
Converting to draft PR, need to handle grouping of filters before reviewing |
|
Filter functionality now works for individual filters |
|
Ready for review @ramonverse |
|
Hi @khareyash05 , unfortunately this blocks the main thread for larger files and the UX is terrible. This feature probably needs to be implemented with a web-worker in order to work ok in the browser. |
Understood. Are we looking for pushing this task to asynchronous processing? I do have an idea around the same?would love to work on the idea, if it isn't in an immediate need. Thanks Also it would be great to hear your ideas on the same. Would help me get a direction |
|
@khareyash05 I'd say make a web-worker with comlink https://github.com/GoogleChromeLabs/comlink that just calculates the rows. Also save the previous rows in a state so we can show it like this in the UI: `${prevRowCount} -> ${currentRowCount)}` |
On it! |
|
@ellipsis-dev review |
|
OK! Reviewing this PR... Responding to this comment by @khareyash05. For more information about Ellipsis, check the documentation. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested.
- Reviewed the entire pull request up to 96f5c65
- Looked at
127lines of code in3files - Took 2 minutes and 29 seconds to review
More info
- Skipped
1files when reviewing. - Skipped posting
0additional comments because they didn't meet confidence threshold of50%.
Workflow ID: wflow_TjcqQkdghQkePVVq
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. We'll respond in a few minutes. Learn more here.
| import * as Comlink from 'comlink' | ||
|
|
||
| export class FilterRowCount{ | ||
| async findFilteredRowCount( |
There was a problem hiding this comment.
Consider passing the necessary arguments directly to the worker method instead of passing a function. This could help avoid potential issues if the function signature changes.
There was a problem hiding this comment.
@khareyash05 want to addreess this?
Fixes #19
Working Filter functionality
Summary:
This PR adds a new feature that displays the number of rows filtered, achieved by updating the
FilterBlockcomponent and creating a new worker for the calculation, and includes the addition ofcomlinkdependency.Key points:
comlinkdependency inpackage.jsonFilterBlockcomponent in/src/components/FilterBlock.tsxto include logic for calculating and displaying the number of rows filtered/src/components/worker.tsfor calculating the number of rows filteredGenerated with ❤️ by ellipsis.dev