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

Fix part #549 : Options Navigation Drawer Domain layer part 1 #548

Merged
merged 41 commits into from
Jan 21, 2020
Merged

Fix part #549 : Options Navigation Drawer Domain layer part 1 #548

merged 41 commits into from
Jan 21, 2020

Conversation

veena14cs
Copy link
Contributor

@veena14cs veena14cs commented Dec 13, 2019 •
edited
Loading

Explanation

This PR is split into 2.

  1. Part 1 focuses on the domain layer work for user preference screen to manage the behaviour of the App.
    In this PR most of the code is added and reused from profile section and need some suggestions on design approach.
  2. Part 2 Fix part #549 :Options low fi Navigation Drawer App module #614 focuses on app module.

Mock

https://xd.adobe.com/spec/e2239cf4-9cde-4c08-5296-25316c1f0a14-9412/screen/2776cbb1-22b0-42c9-b70b-2136bdb361da/OP-Options-Menu-/

Design Doc

https://docs.google.com/document/d/1uylgD2SwxHMYmHgof_-OTlUwriBL5Ps17r0hEWI0icg/edit#

Checklist

  • The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
  • The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
  • The PR follows the style guide.
  • The PR does not contain any unnecessary auto-generated code from Android Studio.
  • The PR is made from a branch that's not called "develop".
  • The PR is made from a branch that is up-to-date with "develop".
  • The PR's branch is based on "develop" and not on any other branch.
  • The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

@veena14cs veena14cs changed the title WIP : Option Screen Fix #549 : UI: Options Navigation Drawer [WIP] Dec 13, 2019
@veena14cs veena14cs changed the title Fix #549 : UI: Options Navigation Drawer [WIP] Fix #549 : UI Options Navigation Drawer [WIP] Dec 13, 2019
@veena14cs veena14cs changed the title Fix #549 : UI Options Navigation Drawer [WIP] Fix #549 : UI Options Navigation Drawer Dec 16, 2019
rt4914
rt4914 suggested changes Dec 16, 2019
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

@veena14cs there are lots of nit changes like spacing issue, naming convention issues, and also android suggestion code optimisation. Please self-review this PR first till then I am unassigning myself.

@rt4914 rt4914 assigned veena14cs and unassigned rt4914 Dec 16, 2019
@veena14cs
Copy link
Contributor Author

@veena14cs there are lots of nit changes like spacing issue, naming convention issues, and also android suggestion code optimisation. Please self-review this PR first till then I am unassigning myself.

Fixed nit issues PTAL.

@veena14cs veena14cs removed their assignment Dec 17, 2019
@rt4914
Copy link
Contributor

rt4914 commented Jan 21, 2020

@veena14cs as per @BenHenning 's comment #574 (review)
you can split this PR.

@veena14cs veena14cs changed the title Fix #549 : UI Options Navigation Drawer[DO NOT MERGE] Fix #549 : Options Navigation Drawer Domain layer Jan 21, 2020
@veena14cs veena14cs changed the title Fix #549 : Options Navigation Drawer Domain layer Fix part #549 : Options Navigation Drawer Domain layer part 1 Jan 21, 2020
rt4914
rt4914 approved these changes Jan 21, 2020
View reviewed changes
Copy link
Contributor

@rt4914 rt4914 left a comment

Choose a reason for hiding this comment

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

LGTM

@rt4914 rt4914 assigned veena14cs and unassigned rt4914 Jan 21, 2020
@veena14cs veena14cs merged commit fd80936 into develop Jan 21, 2020
@rt4914
Copy link
Contributor

rt4914 commented Jan 21, 2020

@veena14cs Please revert this merge, we need @BenHenning 's approval in this PR to confirm whether the approach is correct or not.

veena14cs added a commit that referenced this pull request Jan 21, 2020
@veena14cs
Revert "Fix part #549 : Options Navigation Drawer Domain layer part 1 (…
36b5de5 
…#548)"

This reverts commit fd80936.
This was referenced Jan 21, 2020
Merged
Merged
@veena14cs
Copy link
Contributor Author

@veena14cs Please revert this merge, we need @BenHenning 's approval in this PR to confirm whether the approach is correct or not.

Created duplicate PR of this in PR #617

veena14cs added a commit that referenced this pull request Jan 21, 2020
@veena14cs
Revert "Fix part #549 : Options Navigation Drawer Domain layer part 1 (…
24b5750 
…#548)" (#616)

This reverts commit fd80936.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rt4914 rt4914 rt4914 approved these changes

@nikitamarysolomanpvt nikitamarysolomanpvt Awaiting requested review from nikitamarysolomanpvt

Assignees

@veena14cs veena14cs

@nikitamarysolomanpvt nikitamarysolomanpvt

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@veena14cs @rt4914 @nikitamarysolomanpvt