-
Notifications
You must be signed in to change notification settings - Fork 61
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
[WIP] Add categories to favourites #510
base: master
Are you sure you want to change the base?
Conversation
To create a new column in the database, you also need to increase the if (oldVersion < 7) {
// Add column for tags
db.execSQL("alter table " + STAR_TABLE + " add column tags text");
} While it is possible to store the tags as JSON in a
When creating the second table, you can add The main table The benefit of this is a better database structure and better performance with many items and tags because we don't need to load all items, parse their JSON in the |
Oh, I see. I'll brush up on my SQL and work on this. I'll post a progress report soon once I'm done with the database part. On a side note, two new databases have to be created, so that means I have to create two new classes that extend the |
Not necessarily - we only need two new tables in the database, not a whole new database. E.g. the |
Oh, in that case, we just need to create two new tables in |
Yes :) |
5dfee83
to
5918b13
Compare
Progress update: To-do: |
Yes, I think this makes sense. The |
Now it works! I mean, the text seems to be added only after typing it in the text box as seen from the logs. But the text doesn't appear. I'll try to work it out in the meantime. On a side note, I managed to remove the random text for the tag. Apparently, its because I set the tag to a value first before loading the tag view here. |
Great! 😃 |
431aca0
to
27da86b
Compare
Progress updateCompleted
To-do
Doubts / Errors
|
One suggestion regarding the UI: Maybe it would make more sense to just have one button on every item (e.g. with this icon ) that shows a dialog with the list of currently assigned tags which can be removed with a "delete" button. New tags could then be added with an This probably means that a custom layout would need to be created for the AlertDialog. Regarding the logic in the
This way, the filtering and autocompleting will be easier to implement. |
Progress updateCompleted
To-do
Updates
PS: Sorry for taking so long on this PR! 😅 |
Very nice!
Okay, strange - I'm quite busy at the moment and will be travelling for two weeks starting April 8, so it might take a while until I can take a look at this. But no problem, you can continue with the
I think I would still keep the
Yes, you can use another horizontal
Sorry, I don't understand what you mean with this?
Absolutely no problem - this idea has been sitting around for a few years now, it is not urgent in any way 😉 One more thing that I noticed: this should be changed to |
Ah - I think it's good in the current style - that's the default in Material Design. |
On a side note, I've been quite busy with my final exams. So once I'm done with that, I'll continue work on this! Sorry for the delay! 😅 |
No problem - hope the exams go well! |
Implement add tags functionality
The following were done as well: - Improve add tag functionality to use a list of tags of instead of single tags - Add comments to code
- Implement Filterable interface for ItemListAdapter - Add search bar for fragment_starred.xml - Update tag_menu.xml to place the add button to the right of the edit text
a668cea
to
a3b37c5
Compare
Great! I have just looked at the AutoCompleteTextView problem and fixed it with the commit 17e0463 (the error was just related to the position Regarding the Filter UI: I think a search where you only need to input part of the tag name - it would be enough if you first "search for the tag" in the search field, select it, and then the list is filtered by that tag. I think a nice UI solution for this would be this library: I don't know why the |
I discussed with @raphaelm - probably for the filtering by tag, the best UI would be to add another dialog where one can select one (or multiple) from the list of all tags. The "nachos" library would be nice for the dialog where the tags can be added to the individual items, but the current solution is fine, too. |
Thanks for helping me out with this!
For this, I'm rather confused as to how I could filter by library and tag as well. Since these two details are from different tables, it seems like I would have to do a raw SQL query in order to join the two tables. However, the For your convenience, here is the SO post:
So far, I have got the However, this implementation is rather clunky and results in slow/no update of the favourites list 😅 . So I guess it would be better if I'm able to filter using an As for the nachos library, I'll look into it as soons as I've figured out this filter logic! |
Sorry, I thought this would be easier, I forgot that a join is needed. I haven't had much time to look into this, but I think the two options are either to add this join as a raw SQL query in the StarContentProvider or drop the CursorLoader altogether and use a normal ArrayAdapter instead. |
Progress UpdateSorry for the really late update! I was really busy with my internship and just got some free time 😅. I've formed the raw query now but my question is how do I place it in the |
Nice! Currently the StarContentProvider supports two types of URIs:
As seen in the StackOverflow post that you posted previously, there are two possibilities: either you use your new query with the JOINs every time when the Also, if we don't use a free text input for filtering by tags, but show the list of tags and let the user select from that, you don't need the second JOIN, because then you would just query the database for the selected tag's ID instead of its name. |
- Remove Filterable interface for ItemListAdapter - Remove search bar for fragment_starred.xml - Add an SQL join raw query to filter tags using the tag IDs - Add a dialog for user to select tags to filter with
Progress UpdateI have just completed the functionality for the filter using user selection instead of free text input and I've also removed the second inner join accordingly. Here's the result: I'm not sure if UI is acceptable so I'm open to suggestions! On the other hand, I've imported the "nachos" library and I'll look up on how to add it to the Add Tag dialog. One question on this though, I would just have to replace the |
<item | ||
android:id="@+id/action_filter_by_tags" | ||
android:icon="@drawable/ic_search_24dp" | ||
app:showAsAction="never" |
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.
You could change this to app:showAsAction="ifRoom"
so that it is shown as an action button. The Icon could be changed to the filter icon.
I haven't yet tested it myself and looked at the code in detail, but the UI looks very nice in the video you sent! I added one remark below about the action button. |
Thanks! 😄
I've fixed this and made sure the icon was white so as to be consistent with the menu.
On a side note, I would like to clarify this part! Thanks! |
Ah, sorry, I overlooked this. Yes, so the AutoCompleteTextView would be replaced with the NachoTextView, but also the list above would not be necessary, because the NachoTextView can display both the list of currently assigned tags as well as using it to add new ones - that's its advantage. |
Thanks! Yes, the way you described it sounds like I imagined. One thing that could be added is an "x" icon on the right side of the chips (as seen here) so that they can also be deleted with that instead of just using backspace. Also, similar to the AutocompleteTextView, the chip view should also offer autocomplete when adding a new tag.
Yeah, I see that issue. Normally I would have added a Snackbar with an "undo" button to allow the user to quickly undo the deletion of the tag. However, that probably looks odd inside a dialog, so we would probably have to think about another solution. Or we just leave it, as it should be quite easy to re-add an accidentally deleted tag using the autocomplete function. |
I've been trying to find a way to add a Should another library be used instead? Like the google material library as shown in the link you gave above or this MaterialChipView library. This is how it looks like for now, if implemented using the Nachos library (it doesn't look like it fits 😅 ): |
Hm, yeah, strange, it seems that the nachos library has no way of setting a click listener for the icon, just for the whole chip :/ In the Google Material Design library, the chip component was only added recently, maybe we can use that, there's just not much documentation I could find about how to use it inside an EditText input. Maybe this helps? https://stackoverflow.com/a/51610997 For using the material components library, the compileSdk version of the app first needs to be changed to Android P (28) and the currently used support library versions need to be changed to the new AndroidX libraries (see also here). Probably there are also some adjustments needed to the |
Oh I see. Seems like it would take quite a bit of time to implement that as well.
In the meantime, I'll get started with this and maybe try to look for other solutions if possible! 😄 |
@vivekscl I started with the migration to Android P on the |
Alright I understand! I'll get started on it soon! |
Currently, I've tried to add the tags to the database but when I try to run the app and click on the star button on a book, I get this error (tried to fit the entire error message, but it seems clicking it gives a better view):
I did some searching and it seems I need to install and uninstall the app in the emulator to rebuild the SQL database so that it can support the new column that was added. But I'm unsure of how to go on about doing this because I tried to uninstall it but I'm not sure how to install the debug version of the app.
On a side note, is this good implementation for the tags in the database or does it need more improvement? 😄