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

Make typed email persist between different AuthActivity fragments #982

Merged
merged 1 commit into from May 27, 2018
Merged

Make typed email persist between different AuthActivity fragments #982

merged 1 commit into from May 27, 2018

Conversation

mooocer
Copy link
Member

@mooocer mooocer commented May 26, 2018 •
edited

Fixes #979

Changes:

  1. Typed emails now persist between different screen changes so that the user doesn't need to retype email repeatedly.

Gif for the change:
videotogif_2018 05 26_21 02 30

@codecov
Copy link

codecov bot commented May 26, 2018 •
edited

Codecov Report

Merging #982 into development will decrease coverage by 0.08%.
The diff coverage is 0%.

Impacted file tree graph

@@                Coverage Diff                @@
##             development     #982      +/-   ##
=================================================
- Coverage          29.12%   29.04%   -0.09%     
  Complexity           552      552              
=================================================
  Files                158      159       +1     
  Lines               5329     5344      +15     
  Branches             190      190              
=================================================
  Hits                1552     1552              
- Misses              3712     3727      +15     
  Partials              65       65
Impacted Files Coverage Δ Complexity Δ
...sasia/openevent/app/core/auth/SharedViewModel.java 0% <0%> (ø) 0 <0> (?)
...a/openevent/app/core/auth/login/LoginFragment.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...openevent/app/core/auth/signup/SignUpFragment.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...re/auth/forgot/request/ForgotPasswordFragment.java 0% <0%> (ø) 0 <0> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9711a69...ac25b11. Read the comment docs.

@mooocer
Copy link
Member Author

mooocer commented May 26, 2018

@iamareebjamal I mistakenly worked on navigation, which is being refactored. Should I remove the part about navigation?

@iamareebjamal
Copy link
Member

@MishuVS Yes

@fossasia fossasia deleted a comment May 26, 2018
@mooocer mooocer changed the title Auth fragments: remove need to retype email, fix back presses Make typed email persist between different AuthActivity fragments May 26, 2018
@mooocer
Copy link
Member Author

mooocer commented May 26, 2018

@iamareebjamal Please review

iamareebjamal
iamareebjamal requested changes May 26, 2018
View reviewed changes
app/src/main/java/org/fossasia/openevent/app/core/auth/AuthActivity.java Outdated
@@ -28,6 +28,8 @@
@Inject
BackPressHandler backPressHandler;

String email;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private

...rc/main/java/org/fossasia/openevent/app/core/auth/forgot/request/ForgotPasswordFragment.java Outdated
@@ -42,6 +43,11 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState) {
binding = DataBindingUtil.inflate(inflater, R.layout.forgot_password_fragment, container, false);
validator = new Validator(binding);

getPresenter().getEmailId().setEmail(
((AuthActivity) getActivity()).getEmail()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not safe. Communicate through interface

@mooocer mooocer force-pushed the login#979 branch 2 times, most recently from 9768ea6 to 160dba4 Compare May 26, 2018 16:52
iamareebjamal
iamareebjamal requested changes May 26, 2018
View reviewed changes
...rc/main/java/org/fossasia/openevent/app/core/auth/forgot/request/ForgotPasswordFragment.java Outdated
@@ -42,6 +42,11 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container,
Bundle savedInstanceState) {
binding = DataBindingUtil.inflate(inflater, R.layout.forgot_password_fragment, container, false);
validator = new Validator(binding);

getPresenter().getEmailId().setEmail(
((LoginFragment.EmailIdTransfer) getContext()).getEmail()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still unsafe, no casts should be there

Copy link
Member Author

@mooocer mooocer May 26, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iamareebjamal I checked here and found that SharedViewModel should be used. But they also had cast to their interface when using it, though with try-catch. What was the correct way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explicitly setting interface reference from activity on fragment through newInstance

This solution is better

@mooocer mooocer force-pushed the login#979 branch 2 times, most recently from 487a0b3 to 2bc6478 Compare May 26, 2018 19:41
iamareebjamal
iamareebjamal reviewed May 26, 2018
View reviewed changes
...rc/main/java/org/fossasia/openevent/app/core/auth/forgot/request/ForgotPasswordFragment.java Outdated
@@ -32,6 +34,7 @@

private ForgotPasswordFragmentBinding binding;
private Validator validator;
SharedViewModel sharedViewModel;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you not use private fields?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, inadvertently got left out.

@iamareebjamal iamareebjamal merged commit 3c22d28 into fossasia:development May 27, 2018
@mooocer mooocer deleted the login#979 branch May 27, 2018 12:56
iamareebjamal pushed a commit that referenced this pull request Jul 16, 2018
@mooocer @iamareebjamal
feat: Persist typed email between different AuthActivity fragments (#982
84fc574 
)

- Use SharedViewModel
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iamareebjamal iamareebjamal iamareebjamal approved these changes

Assignees

@mooocer mooocer

Labels
ready-to-ship
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mooocer @iamareebjamal @robuxinc