CCExtractor / beacon

Flutter application to share location with a group. (under development)
58 stars 148 forks source link

Pull to refresh feature with UI changed #175

Closed Rushour0 closed 1 year ago

Rushour0 commented 1 year ago

Fixes # Added a custom pull to refresh mentioned in the issue #91 I have also made UI changes to the auth screen mentioned in the issue #158

Describe the changes you have made in this PR - Issue 91 Made a new TabControllerStorage since we cannot late initialise the tabcontroller, and thus cannot declare it out of the build method. Storing the tab index using this singleton implementation. Added a new package custom_refresh_indicator to implement the pull to refresh feature with a custom icon

Issue 158 Made changes to the UI to make it better looking and user friendly.

Screenshots of the changes (If any) - Issue 91

https://user-images.githubusercontent.com/72869428/198694578-2ddde619-25c0-4ba7-b101-97f0d227f297.mp4

Issue 158 login signup

Rushour0 commented 1 year ago

@ItsAdityaKSingh you can check this commit, made a new branch for merging changes into it. @nb9960 You can also check this if the need be.

Rushour0 commented 1 year ago

Excellent work, @Rushour0! Overall looks good to me, though it partially closes issue #91. It still requires to be refreshed rather than automatically refreshed.

For the UI part, the buttons could be remade into a bit circular; it looks boxier than it should be on the hike and group screen. Also, for the auth screen, keeping the fields in the same card would still be preferred, honestly, though we can improvise. And the elevation also could be reduced.

As for the constant refresh part, I feel it will consume more data to constantly keep refreshing the data. If people are going for a hike, then the connectivity and internet might be an issue. Considering that I put a pull-to-refresh instead of a stream like constant refresher

ItsAdityaKSingh commented 1 year ago

As for the constant refresh part, I feel it will consume more data to constantly keep refreshing the data. If people are going for a hike, then the connectivity and internet might be an issue. Considering that I put a pull-to-refresh instead of a stream like constant refresher

It should not always be refreshing, but as of right now, when the timer hits 0, the whole list needs to be manually refreshed to see that change. I meant to refresh the card when it becomes active. It could be automated to refresh only the card and not the whole list when it becomes active. Are you getting the idea?

Rushour0 commented 1 year ago

As for the constant refresh part, I feel it will consume more data to constantly keep refreshing the data. If people are going for a hike, then the connectivity and internet might be an issue. Considering that I put a pull-to-refresh instead of a stream like constant refresher

It should not always be refreshing, but as of right now, when the timer hits 0, the whole list needs to be manually refreshed to see that change. I meant to refresh the card when it becomes active. It could be automated to refresh only the card and not the whole list when it becomes active. Are you getting the idea?

Ahh alright 😌, I see what you wanted to do here. Okay, so essentially instead of complete list, just refresh the card according to its timer. If this is correct then I will work on this after my academic exams are over. Will get back to you in a few weeks with the new code.

ItsAdityaKSingh commented 1 year ago

Yup, you got it right! Along with this pull-to-refresh feature, there was this to be done! And indeed, you can work on it as soon as you get over the exams :)

Rushour0 commented 1 year ago

Yup, you got it right! Along with this pull-to-refresh feature, there was this to be done! And indeed, you can work on it as soon as you get over the exams :)

Alright then, I will finish the earlier changes mentioned in the code review and submit with the refreshing feature

ItsAdityaKSingh commented 1 year ago

Any updates @Rushour0?

Rushour0 commented 1 year ago

Any updates @Rushour0?

Was travelling a few days and was ill for some time. Just restarted work today. Will be done in 2-3 days.

Rushour0 commented 1 year ago

Any updates @Rushour0?

Hey I am facing a problem I am not able to push any updates to the rushour0 branch of my repository. It says error: failed to push some refs

Can you help me out?

@ItsAdityaKSingh I am actually dying here, because I spent 20 minutes making changes 2 hours trying to push a commit. Somehow my repository is corrupted(?) I actually do not know what to do here.

but to show you that change you suggested of rotating refresh icon

refresh_loading

I have also made all other changes in the borders and all but I am not able to push anything.

Rushour0 commented 1 year ago

Any updates @Rushour0?

Hey I am facing a problem I am not able to push any updates to the rushour0 branch of my repository. It says error: failed to push some refs

Can you help me out?

@ItsAdityaKSingh I am actually dying here, because I spent 20 minutes making changes 2 hours trying to push a commit. Somehow my repository is corrupted(?) I actually do not know what to do here.

but to show you that change you suggested of rotating refresh icon

refresh_loading

I have also made all other changes in the borders and all but I am not able to push anything.

Was still facing issue, uploaded the upgraded files manually, pulled and did a flutter test to verify

Rushour0 commented 1 year ago

@ItsAdityaKSingh can you review this again?

Rushour0 commented 1 year ago

@ItsAdityaKSingh hey can you check the latest changes in the PR

ItsAdityaKSingh commented 1 year ago

Hey @Rushour0! Sorry I was engaged with other stuff. Will review and get this resolved soon. Appreciate the patience :) Some tests are failing, could you fix and commit them?

ItsAdityaKSingh commented 1 year ago

Also, the issue you were facing with the commit and pull, it's fixed right now, right?

Rushour0 commented 1 year ago

Also, the issue you were facing with the commit and pull, it's fixed right now, right?

No. I want to smash my repository. Idk what the issue is. It just keeps denying any remote push

Rushour0 commented 1 year ago

Hey @ItsAdityaKSingh can you check if this can be merged?

Rushour0 commented 1 year ago

Opening a new PR for the same This one has gotten a bit messy and unattended for a while now