search

code4romania / teacher-workout-android

Teacher Workout Android app
Mozilla Public License 2.0
5 stars 17 forks source link

Add UI scaffold for account related screens #17

Closed lukstbit closed 3 years ago

lukstbit commented 3 years ago

What does it fix?

This PR implements the basic UI scaffolding for account related screens. There are still visual tweaks to be made but those need to be implemented in the theme in another PR. Also, the last screen for resetting the password isn't yet implemented, this will be discussed in the appropriate issue.

How has it been tested?

There is nothing to test.

alexandru-calinoiu commented 3 years ago

Thank you for your submission, before merging it will be great if you could review https://github.com/code4romania/teacher-workout-android/pull/18

The pr proposes an app arch that should streamline collaboration, but will require some refactoring this pr, mostly moving code around.

Also we where discussing to use jetpack compose for the UI. We did not do a very good job at communicating this, planning to do a pr to the Readme with the proposed frameworks and patterns.

lukstbit commented 3 years ago

I have no problems writing the UI in compose, I wrote it using normal views because compose wasn't mentioned anywhere. Compose is still in beta but with the release of android 12 it will probably get to 1.0.0(or soon after the release) so we will have a stable release.

We did not do a very good job at communicating this, planning to do a pr to the readme with the proposed frameworks and patterns.

This will be really helpful, to also clarify things like threading

alexandru-calinoiu commented 3 years ago

I have no problems writing the UI in compose, I wrote it using normal views because compose wasn't mentioned anywhere. Compose is still in beta but with the release of android 12 it will probably get to 1.0.0(or soon after the release) so we will have a stable release.

We did not do a very good job at communicating this, planning to do a pr to the readme with the proposed frameworks and patterns.

This will be really helpful, to also clarify things like threading

I've opened a pr for the readme with some tech https://github.com/code4romania/teacher-workout-android/pull/19

lukstbit commented 3 years ago

I've created the basic UI for the account related screens. Because the figma design doesn't have a mockup for showing the outcomes for registration/signing in/resetting password to the user I used a simple panel with information like in the images below. Personally I think a dialog would be more suited.

registration_loading1 registration_failure1 registration_success1

AlexandraDamaschin commented 3 years ago

@lukstbit Sorry for the delay, we merged Calin's PR and I've created a new one with some improvements based on your review which hopefully will be merged soon.

When you get a chance to rebase this PR let us know and will make CR.

As for the outcomes, I would agree with you for adding dialogs in order to let the user know what happened, but I am not the right person to make the final decision. Maybe @catileptic can help us to decide this or to tell us to who we should speak in order to decide?

catileptic commented 3 years ago

@MihaiPantiru, have you, in the iOS app, already implemented these screens? And, if so, have you decided what element to use in order to tell the user whether their actions were successful or failed with an error? I think the most important part is for the iOS and Android apps to be consistent, so if the iOS one already made this decision, let's mirror it. If not, @MihaiPantiru, what is your opinion on the element that we should use?

MihaiPantiru commented 3 years ago

@catileptic @vladstanescu94 implemented for iOS and he added the errors like we have on Figma, inline red text below fields and on top red text for general errors. 277E1005-23CF-43AC-B1AB-C754A0C43952

lukstbit commented 3 years ago

@AlexandraDamaschin I rebased. Note that I had to disable kapt because it throws some compiler errors. I don't know if it's something on my side or kapt has issues with dynamic feature modules/compose but I couldn't make the code compile without disabling it.

@MihaiPantiru I implemented the errors as shown in the design mockups. The question was about what to show to the user after clicking to (create an account/signing in or resetting the password) to show loading and the result of the action(like failure to create an account etc). Above, I've attached some images with my current implementation where I disabled the inputs and then displayed a panel inline with the information. I was thinking about using a blocking dialog so the user it's kind of forced to wait to see the outcome of creating an account/ resetting the password etc. I was wondering how the iOS app handles this.

AlexandraDamaschin commented 3 years ago

@lukstbit Thanks, will look over it. @vladstanescu94 Maybe you help us with more information about how you handled the cases after the user clicks to create an account to show the loading/result of the action? (See detailed question :top: )

lukstbit commented 3 years ago

@AlexandraDamaschin Ok. Is anyone working on the other added UI related issues? If not I can work on them.

AlexandraDamaschin commented 3 years ago

@AlexandraDamaschin Ok. Is anyone working on the other added UI related issues? If not I can work on them.

Yeah, me and one colleague are working on the onboarding steps, except from that nothing I am aware of. We are having a meeting this Saturday with people coding on, so we will work on the issues which don't have any comments.