0xf104a / NextcloudServices

Android app to send notifications from Nextcloud without using GCM(Google cloud messaging)
GNU General Public License v3.0
63 stars 11 forks source link

Reverted UI-Update #37

Open newhinton opened 2 years ago

newhinton commented 2 years ago

Hi, i wanted to fix a small thing that bugged me, but then i saw you reverted many of the changes that were made in the past. Is there a reason for this?

Eg.: fa28201b196816511bb87694ec582a8a7ebd3b4a

0xf104a commented 2 years ago

Hello. I have reverted changes mostly because they are not yet production ready(e.g. when loggining-in at settings fragment it is not synchronized with an upper bar) and currently I have got not enough time to focus on them. So the changes are moved to branch ui-overhaul, so I can fix some small problems in version with older UI and when I will get a bit more time I will fix new UI

newhinton commented 2 years ago

i would love to help out since i created the ui, wouldn't it be more reasonable to keep the changes on master and create tickets for stuff that does not work? That way i can work on those tickets. (Feel free to assign/tag me if appropriate)

To me it seems that this will create quite a bit of merge issues down the road.

0xf104a commented 2 years ago

The problem is that this issues affecting a user experience and main branch is used for F-Droid builds. So in case if those changes are kept on main I would be unable to fix some small issues without pushing non-production ready changes to F-Droid(and so affecting users).

But yes, unfortunately, such an approach creates a merge conflicts which need to be resolved. So I would merge my changes from a main branch to ui-overhaul and then create a pull request from ui-overhaul and describe a set of issues which we need to fix/discuss

newhinton commented 1 year ago

@Andrewerr Could i move this forward? If i know what needs to be done, i can implement/fix it. I would really like to see this implemented again!

0xf104a commented 1 year ago

@newhinton hello. I think since the new design pardigm in Android is Material3(aka MaterialYou) we should redesign the app in Material3 style. You can start from redesigning current app to MaterialYou:

I think this is a good task list to start from. Further as I think when I would implement multi-account support from #1 we would need to split this preference into multiple activities where there would be "General settings" and per-account settings(such as notification filtering, etc.)

newhinton commented 1 year ago

Thaaat is not a small task. :D Also i disagree a bit with you, depending on what you actually want.

Do you want to fully redesign your app with only the Material3-style, as stated by google, or do you want to follow nextcloud's interpretation of m3?

I highly suggest we keep in line what nextcloud notes&deck do, so that we have a consistent design-line. (Granted, they are not updated to m3, and i dont know if that hasnt been changed on their respective repositories)

Alternatively we could follow the nextcloud-app, but only implement the base-blue color in the first run, because dynamic colors are hard.

Also in any case, i would not like to restart from the current master-branch, there is already quite a bit of work done in my past branches, and they serve as a good base, and even an intermediary-version if needed be.

Additional changes should be made on top of that.

newhinton commented 1 year ago
1 2 3

This is the current state of the ui-branch. I dont think we should start from the current master branch, instead i think it would be best to update the ui-one to m3, otherwise we have to re-implement the whole ui, and a lot of features with it.

Edit: I have updated dependencies in my own ui-branch, if you want to check it out. Because the master has moved on, should i rebase the ui-one so that we can move on from there?

0xf104a commented 1 year ago

Sorry for a long response. Actually I have found few problems in your UI:

Yes it would be needed to rebase but wait until I would revert my own UI-changing commits. I will tag you here when main branch would be ready to be rebased into yours

newhinton commented 1 year ago

As a user I don't really understand what that crossed cloud(on the left from profile photo) means, since it is always crossed

That can be removed, it should display the state of the connection, but may not be nessessary.

Homepage is empty

True, but in the future i'd like to see past notifications there.

There is no way to really understand what status of app is right now

That is what the cloud-icon is supposed to convey ;)

Login is a bit counter-intuitive.

Okay i forgot how deck&notes handle this. We should do it the same way, with an initial login-activity.

I think we can fix a lot of the issues you mentioned relatively easy with the initial work. When you tag me, i'll start work on those!

0xf104a commented 1 year ago

@newhinton okay I think you may rebase latest changes to your fork, I have updated everything. I also would do some further changes in a notification backend, but would try not to touch anything UI-related further to reduce merge conflicts

newhinton commented 1 year ago

Okay a normal rebase is not working right, all changes from the ui are lost in either automatic merge or rebase. I will try to do it manually, sadly.

0xf104a commented 1 year ago

Merging would be more convenient since you need to fix conflicts only once, while rebase applies each commit which is not in your branch(so you fix conflicts for each commit)