HaikuArchives / Calendar

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

UI Part for Filter & Search Utility #120

Closed harshit-sharma-gits closed 2 years ago

harshit-sharma-gits commented 2 years ago

I have made the UI Part of the Filter & Search Issue #119, and it is looking like this

Changes Summary:

Also, all the clickables present in the UI, currently sends a tempMessage that I created, and it is not functional yet

harshit-sharma-gits commented 2 years ago

If after using the app a bit, it turns out more fine-grained filtering needs a category pop-up menu, there's no problem adding it. But to start out needlessly complicated...

Let's stick to one feature at a time, the filtering, and leave "profiles" until this one is working.

Okay, so should I remove the unnecessary things, that is, Profile and Categories MenuFiled. And leave "Search" Text Control and the clear button there?

Well I don't think having the Clear button would be a nice thing, as only a Search Box is present, and if we implement a category popup menu in future, then we may have the Clear button as well.

humdingerb commented 2 years ago

Yes, as I wrote in #119, just the BTextControl labelled "Filter:". No "Clear" button. It'd be nice to capture ESC to clear the filter. But that can come later.

nielx commented 2 years ago

I added some comments throughout the code.

Two general remarks about PR hygiene:

  1. Your changes are spread over three commits. While I can imagine you make these commits internally while working on it and wanting to keep the changes, when you are going to upstream these changes you generally want to clean them up, i.e. squash them into one commit. You also want to give them clear, concise commit messages about the change. 'Get Started' is not one of those.
  2. You are adding the Calendar.pld file. It is fine to use this locally, but it should not be added remotely. The official build method is to use the Makefile, so we do not need additional IDE files for this. Please remove this from the commit.

Furthermore, I agree with @humdingerb about keeping it simple. As we discussed on our call on Sunday, let's start with text filtering. We can add the other things later.

humdingerb commented 2 years ago

Two general remarks about PR hygiene:

WRT working on PRs in general, I recommend using "feature branches". There's a nice intro to it at https://github.com/haikuports/haikuports/wiki/DevelopmentModel That way, you can easily leave your work in one branch for a bit and do some experimenting in another.

It's also a good idea to separate your work into distinct commits, e.g. do style fixes in one commit, GUI work in another and backend work in yet another. Before creating your PR, do a "git -i rebase" to move those commits around and squash the commits of a similar area.

humdingerb commented 2 years ago

Anything new, @harshit-sharma-gits ? It's been very quiet for too long...

harshit-sharma-gits commented 2 years ago

Hello! As mentioned, I have created a new branch, namely, filterFeature, for implementing the Filter Utility And cleared the excess clutter from the main window, now the updated UI looks like this -

UI Part Refined

Now, the current goal for the filter is to capture the keywords, and search them in the events list for the particular tab (Day, Week, or Month) and display them accordingly.

humdingerb commented 2 years ago

You haven't pushed that branch to your repo, it appears. If you update this PR, we can keep track of your progress. Just adding a BTextControl to the window seems a bit scant for a week of full time work... Do speak up when you're stuck somewhere and need some pointers.

nielx commented 2 years ago

Superseded by PR #121