search

enviroCar / enviroCar-app

enviroCar Android Application
https://envirocar.org
GNU General Public License v3.0
88 stars 154 forks source link

Add bin icon to delete car in my cars screen #922

Closed Jdbarad closed 1 year ago

Jdbarad commented 1 year ago

https://user-images.githubusercontent.com/93068379/175763089-17575158-b158-4468-93c8-eb734e8dba37.mp4 @SebaDro

Jdbarad commented 1 year ago

@SebaDro sir Review this PR.

SebaDro commented 1 year ago

Thanks @Jdbarad for your revision. I really like the text marquee feature, so I'm willing to accept it. However, just consider some small adjustments before merge:

  1. Rebase your feature branch from develop if not already done
  2. Change your pull request so that it will be merged into develop
  3. Keep the context menu for deleting and selecting single cars that will pop up when clicking on a single car row
  4. Add the full car name to the context menu. This will show users the full car name without having to select a car for activating the marquee feature.

To implement the last point, just set a title and content on the existing MaterialDialog.Builder. If you want to, you can also replace the com.afollestad.materialdialogs.MaterialDialog by SimpleDialog which uses the com.google.android.material.dialog.MaterialAlertDialogBuilder. We introduced the MaterialAlertDialogBuilder some time ago, so this step will unify the UI a bit more.

Jdbarad commented 1 year ago

@SebaDro Sir, New PR #924 is Ready for Review.

SebaDro commented 1 year ago

Please, just update this PR in accordance to the requested changes, in order to avoid opening too much PRs for the same feature. You can change the branch to merge in when editing the PR and push changes to the Jdbarad:share branch. New commits pushed to Jdbarad:share will automatically apply to this PR.

Jdbarad commented 1 year ago

https://user-images.githubusercontent.com/93068379/175918733-d1c59131-e353-46a8-9287-7b21928dbbf3.mp4

@SebaDro Sir Now Everything is Updated.

SebaDro commented 1 year ago

Well, the text marquee feature should only be active for the selcted car. Otherwise the user would be distracted by too many animations. By clicking on a car item, the user now can see the full car name in the MaterialDialog. That's quite sufficient. So please, just consider a last revision.

Jdbarad commented 1 year ago

@SebaDro Sir, PR Updated

SebaDro commented 1 year ago

I think you forgot to push it.

Jdbarad commented 1 year ago

I think you forgot to push it.

@SebaDro Sir Updated