HaikuArchives / Calendar

:calendar: A native Calendar application for Haiku.
MIT License
29 stars 20 forks source link

Added a TextControl with Apply and Clear buttons #121

Closed harshit-sharma-gits closed 2 years ago

harshit-sharma-gits commented 2 years ago

Apart from adding the above mentioned. Also, using message, I tried to clear out the TextControl with SetText(NULL) whenever the user presses the Clear Button, but it didn't seem to be working.

harshit-sharma-gits commented 2 years ago

Hello! I just tested out a function for filtering the events, searchForKeywords().

The idea is to to get the details of a particular event, including Name, Place & Description, in a string. (So that the keywords can be searched in all of the three)

Then the keywords are searched in the event details. If any one of the keyword is found in the event details (irrespective of the case), then that event will be shown.

The filtering is working fine, but currently, to-be-searched keywords are not fetched by the Filter TextControl (need help regarding that), but hardcoded as char* toSearch

Also, I thought having the filtering done in QueryDBManager would be a great fit, because the events are directly loaded from File there. Any suggestions on this?

humdingerb commented 2 years ago

I defer to @nielx here, but I wonder if it wouldn't be better to do the filtering in EventTabView::_PopulateList(). When the filter string changes, you just have to trigger a _PopulateList(), while in QueryDBManager, you'll have to trigger a BQuery (which in turn tiggers a _PopulateList()).

I'd include the category in the filtering.

nielx commented 2 years ago

I defer to @nielx here, but I wonder if it wouldn't be better to do the filtering in EventTabView::_PopulateList(). When the filter string changes, you just have to trigger a _PopulateList(), while in QueryDBManager, you'll have to trigger a BQuery (which in turn tiggers a _PopulateList()).

I do not have a strong opinion on either approach. A fundamentalist view is that we use the database to perform querying, and thus these filters should be applied to the database query (database = filesystem in this case). A pragmatic view is that can do it locally, we do not need to load new data, we just do work on the current data. From a user perspective, I also wonder if it makes sense to show some additional stats that make clear how many items are filtered, much like in Excel you have this little bit in the status bar showing how many records of which total are visible when you use filters. But that would be for future iterations.

nielx commented 2 years ago

@harshit-sharma-gits : if you make changes based on our comments, could you click on the Resolve button in this thread to mark which one you have completed?

nielx commented 2 years ago

Hello! I just tested out a function for filtering the events, searchForKeywords().

The idea is to to get the details of a particular event, including Name, Place & Description, in a string. (So that the keywords can be searched in all of the three)

The overall approach looks sound, though I will do a more thorough review if you go for this one.

However, if you decide to implement this at the QueryDB level (see other comment), then I would argue that you should probably use the file systems query features to do this search. See the query.Push*() calls above on where it should factor in. A good pointer on what the query system can do can be found in the user documentation.

Let us know what you decide and then we can review this part of the code.

harshit-sharma-gits commented 2 years ago

Hello! I've moved the filter function to EventTabView's _PopulateList() function. The reason being : If we add the filter at QueryDBManager level, then we would need to access the files every time, whenever the Filter Keywords are changed. And that would not be much efficient. Instead, I think filtering out an existing list fetched from the QueryDBManager would be efficient. The aim is to get the keywords from the TextControl and pass it to _PopulateList() where the filtering will take place.

But, I'm still stuck at sending the kFilterCleared Message. Need suggestions.

humdingerb commented 2 years ago

But, I'm still stuck at sending the kFilterCleared Message. Need suggestions.

You shouldn't have let the clear-button-issue distract you too much. It's a side show, more important would be to get the contents of the fiter BTextView to where it's needed. However, there's the "AttachedToWindow()" hook function of BView where a SetTarget() might work to fix the apply button messaging.

harshit-sharma-gits commented 2 years ago

Hello! I've added the hooks from SidePanelView and EventTabView to the MainWindow and, now the Filtering seems to be working nicely; You may also see it here

harshit-sharma-gits commented 2 years ago

You shouldn't have let the clear-button-issue distract you too much. It's a side show, more important would be to get the contents of the fiter BTextView to where it's needed. However, there's the "AttachedToWindow()" hook function of BView where a SetTarget() might work to fix the apply button messaging.

I have tried this, still no improvement. But, a rather off-chart approach worked!

Made a EmptyFilterText() public function in SidePanelView. Which, as the name suggests, clears the TextControl. And called this function from the MainWindow when the kFilterCleared case happens, like this :

case kFilterCleared:
    fSidePanelView->EmptyFilterText();
    break;

And voila, It works like a charm!