Skip to content
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.

Sign up for GitHub

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

Jump to bottom

This pull request enables new group widgets #669

Draft
wants to merge 52 commits into
base: main
Choose a base branch
from
Draft

This pull request enables new group widgets #669

wants to merge 52 commits into from

Conversation

Altonss
Copy link
Contributor

@Altonss Altonss commented Dec 10, 2021 •
edited
Loading

This is just a draft because after a group widget is created, the group doesn't open up. Instead just the last window of the app opens. The group id is well transfered with the intent to the MainActivity, but then I don't really know how to do ^^ I'm sure it's a not a very difficult fix, but for the moment I'm stuck.
This would fix this feature request: #397

Altonss added 20 commits December 8, 2021 00:31
Add code 93
7fca6db
Merge remote-tracking branch 'origin/master'
e727a27
Fix long press to add card to group
8c86fa8
Merge remote-tracking branch 'origin/master'
a15cc31
Merge remote-tracking branch 'origin/master'
4304a51
First step to implement group widget
7cf8d89
Try to fix (try with a standard list view)
2504a24
Try to fix with group cursors
4a5f517
Blank list, fix with GroupCursorAdapter
1bfff05
implement GroupCursorAdapter
a8f9baf
implement click action
a6aa6f6
try fixes with renaming groupListListItemViewHolder
10da2a7
try fixes with onResume group view
5a074db
try fixes with updateLoyaltyCardList()
8078f0c
try fixes with groupTabs selectTab
d57f26e
fix, just set selectedTab
d3c2412
Revert some changes to see if the app starts again correctly :/
30f1381
Select group for widget
36cdbd1
fix select button class
7563bee
Merge branch 'master' of https://github.com/TheLastProject/Catima int…
004df4a 
…o group-widget-2

� Conflicts:
�	app/src/main/res/values/strings.xml
@TheLastProject
Copy link
Member

In MainActivity there is a note like this:

// Restore settings from Shared Preference

You probably need to edit that to be like: if this is opened from the group widget, use that group. Otherwise, restore from shared preference

@Altonss
Copy link
Contributor Author

Altonss commented Dec 10, 2021

Ah ok I understand. But I don't really know how to write the "if this is opened from the group widget" logic... I'm new to android development :)

@Altonss
Copy link
Contributor Author

Altonss commented Dec 10, 2021

It works now on my device! Feel free to make a review or propose some changes. As I'm new to android development, don't hesitate to make suggestions/corrections/bug reports !

@Altonss Altonss marked this pull request as ready for review December 10, 2021 18:00
@Altonss
Copy link
Contributor Author

Altonss commented Dec 10, 2021 •
edited
Loading

I noticed a bug: when changing a group name after a widget has been created for this group, the widget doesn't work anymore.
At least like for the cards, the widget should continue to work even if there the widget doesn't update the icon and the text.
EDIT: this also happens for deleted groups. The same functions as the card shortcuts need to be implemented.

@TheLastProject
Copy link
Member 

android.database.sqlite.SQLiteOpenHelper.getReadableDatabase(SQLiteOpenHelper.java:187) at protect.card_locker.CardShortcutConfigure.<init>(CardShortcutConfigure.java:21)

That's the important part, so CardShortcutConfigure.java, line 21. It doesn't look like you changed anything there so maybe you found a crash that's in the master branch? What did you do to reproduce it? I can't reproduce the crash you mentioned on the master branch. I haven't tried your branch yet for this, it's been a busy week sorry.

@Altonss
Copy link
Contributor Author

Altonss commented Dec 16, 2021 

android.database.sqlite.SQLiteOpenHelper.getReadableDatabase(SQLiteOpenHelper.java:187) at protect.card_locker.CardShortcutConfigure.<init>(CardShortcutConfigure.java:21)

That's the important part, so CardShortcutConfigure.java, line 21. It doesn't look like you changed anything there so maybe you found a crash that's in the master branch? What did you do to reproduce it? I can't reproduce the crash you mentioned on the master branch. I haven't tried your branch yet for this, it's been a busy week sorry.

No worries, you can take your time :) I don't think I changed anything in this place compared to master. Will try the master branch on the same device and maybe open an issue if I encounter the problem again. To reproduce the issue on my branch: just try to create a shortcut.

@Altonss Altonss mentioned this pull request Dec 16, 2021
Closed
@Altonss
Copy link
Contributor Author

Altonss commented Dec 16, 2021

It's the same in the main branch, just created an issue.

@Altonss
Copy link
Contributor Author

Altonss commented Jan 9, 2022

I'm sorry that this takes so much time for me to fix. Currently I don't have much time, but I plan to resume the work on this PR soon:copyright: :laughing:
But I anyone has an idea of how to help for the update/delete shortcut functionnality it would be great. I tried several things but it was not working :/
Thanks!

@TheLastProject
Copy link
Member

No worries. I could maybe do the update/delete part myself if you can't run an emulator with a less broken Android version but no promises because time and stuff

Altonss added 2 commits March 27, 2022 19:12
Merge remote-tracking branch 'origin/master' into group-widget-2
b5dae66 
# Conflicts:
#	app/src/main/java/protect/card_locker/MainActivity.java
#	app/src/main/res/values/strings.xml
Merge current master branch into this branch, to keep it up to date w…
bc9fc4f 
…ith latest features
@Altonss
Copy link
Contributor Author

Altonss commented May 25, 2022

Well it looks like I need to update a lot of my code with the new UI... Is there a "rapid" and "clean" way to update to the new UI?

@TheLastProject
Copy link
Member

Looking at the "Resolve conflicts" button on GitHub, the conflicts are really small, should be easy to resolve in the web UI.

But you can also use the command line, look at the text "Use the web editor or the to resolve conflicts." GitHub shows on this PR

@Altonss
Copy link
Contributor Author

Altonss commented May 25, 2022

Yes I now I could use this, but the group widget selector was based on the old UI and has therefor not been adapted to the new UI...

@TheLastProject
Copy link
Member

Hmm, sorry, no suggestion there :(

@Altonss
Copy link
Contributor Author

Altonss commented May 25, 2022

Hmm, sorry, no suggestion there :(

All right, no problem :) Just wanted to ask in case there was a way!

@Altonss Altonss mentioned this pull request Jul 24, 2022
Open
@Altonss Altonss mentioned this pull request Sep 13, 2022
Closed
Altonss added 2 commits September 21, 2022 15:59
Fix bugs
02f2ab1
Merge remote-tracking branch 'origin/master' into group-widget-2
aa53837 
# Conflicts:
#	app/src/main/res/values/strings.xml
@Altonss
Copy link
Contributor Author

Altonss commented Sep 21, 2022

This PR now works again ;) But the issue with the group shortcut update isn't resolved and I'm really stuck. I think the logic for card updates is in ShortcutHelper, but I don't really understand it...

ping @TheLastProject, do you know how I could do this?

@TheLastProject
Copy link
Member

Yeah, I think it is in ShortcutHelper too but I don't know for sure either. That code is still from Loyalty Card Keychain and I never touched it because it always worked for me.

Have you tried running an emulator of a newer Android version than Android 5 in Android Studio yet? Because the last testing we did did seem to show that it's an Android 5 issue also with existing shortcuts.

I think something went wrong with merging the new code into this PR by the way, I see a lot of changes in it unrelated to new group widgets :(

@Altonss
Copy link
Contributor Author

Altonss commented Sep 21, 2022 •
edited
Loading

Yeah, I think it is in ShortcutHelper too but I don't know for sure either. That code is still from Loyalty Card Keychain and I never touched it because it always worked for me.

The code seems pretty "complicated" for me ^^

Have you tried running an emulator of a newer Android version than Android 5 in Android Studio yet? Because the last testing we did did seem to show that it's an Android 5 issue also with existing shortcuts.

I just tested on Android 13 ;) What doesn't work:

I think something went wrong with merging the new code into this PR by the way, I see a lot of changes in it unrelated to new group widgets :(

Really? I just checked again and it seems to me that all changes are related to group shortcuts (except a few blank lines here and there that I could clean up (maybe they were added by the ide?)).

@Altonss
Copy link
Contributor Author

Altonss commented Sep 22, 2022

@TheLastProject After a bit of thinking, I have an idea why the updating might not work:
For cards, the id always remains the same, no? Because if this is the case, then maybe it doesn't work for groups because the id changes it the name of the group changes...

@TheLastProject
Copy link
Member

For cards, the id always remains the same, no? Because if this is the case, then maybe it doesn't work for groups because the id changes it the name of the group changes...

I completely forgot I coded it that way! Ouch, that's not great.

Okay, so, we'll first have to (and let's do this in another PR):

  1. Edit the LoyaltyCardDbGroups table to contain a new ID column that's an integer.
  2. Edit the LoyaltyCardDbIdsGroups table to refer to the group ID instead of group name.
  3. Migrate all current memberships to the new setup.

I think the easiest way is to just create 2 temporary tables and then create the new stuff like I did at newVersion 10 in the DBHelper class.

It's going to be a bit of a pain but a requirement to make this work well I'm afraid :(

@Altonss
Copy link
Contributor Author

Altonss commented Sep 23, 2022

For cards, the id always remains the same, no? Because if this is the case, then maybe it doesn't work for groups because the id changes it the name of the group changes...

I completely forgot I coded it that way! Ouch, that's not great.

So think that this issue might be the cause of the widget not updating because the id changes?

Okay, so, we'll first have to (and let's do this in another PR):

1. Edit the `LoyaltyCardDbGroups` table to contain a new ID column that's an integer.

2. Edit the `LoyaltyCardDbIdsGroups` table to refer to the group ID instead of group name.

3. Migrate all current memberships to the new setup.

I think the easiest way is to just create 2 temporary tables and then create the new stuff like I did at newVersion 10 in the DBHelper class.

It's going to be a bit of a pain but a requirement to make this work well I'm afraid :(

Maybe I can open a new issue to track those 3 points :)

@TheLastProject
Copy link
Member

So think that this issue might be the cause of the widget not updating because the id changes?

Yeah, I'm pretty sure that's the reason. Nice find :)

Maybe I can open a new issue to track those 3 points :)

Sounds great :)

@Altonss Altonss mentioned this pull request Sep 23, 2022
Open
3 tasks
@TheLastProject TheLastProject marked this pull request as draft November 22, 2022 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TheLastProject TheLastProject TheLastProject requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Altonss @TheLastProject