Skip to content
This repository has been archived by the owner on Jul 25, 2024. It is now read-only.

Restore narrow and scroll after theme switch. #414

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jainkuniya
Copy link
Member

Fix:#406

Summary of changes

Now narrow and scroll is restored after switching theme.

Screenshots
Switching theme in homeView
ezgif com-video-to-gif 2

Switching theme in narrowedView
ezgif com-video-to-gif 3

Switching theme in narrowedView and coming back to home
ezgif com-video-to-gif 4

Please make sure these boxes are checked before submitting your pull request - thanks! Guide

  • Code is formatted.

  • Run the lint tests with ./gradlew lintDebug and make sure BUILD is SUCCESSFUL

  • If new feature is implemeted then it is compatible with Night theme too.

  • Commit messages are well-written.

@jainkuniya
Copy link
Member Author

resolved conflicts

@Sam1301
Copy link
Member

Sam1301 commented Mar 27, 2017

@vishwesh3 thanks for working on this! 👍
Faced one issue: When I tried the changes, on switching the theme, the load position of message is not maintained in the narrow.
Heres a short video https://drive.google.com/open?id=0B4jK5QX65b0ZV2xzTFR2d2pyYjQ

@@ -133,6 +134,19 @@ public void setZulipActivity(ZulipActivity zulipActivity) {
this.zulipActivity = zulipActivity;
}

//store narrowFilter before switching theme
private NarrowFilter narrowFilter = null;
Copy link
Member

Choose a reason for hiding this comment

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

default value will be null

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, its null private NarrowFilter narrowFilter = null;

Copy link
Member

Choose a reason for hiding this comment

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

So we won't need to assign null additionally
private NarrowFilter narrowFilter;

@jainkuniya
Copy link
Member Author

Updated

ezgif com-video-to-gif 14

@@ -147,6 +147,11 @@ public void setZulipActivity(ZulipActivity zulipActivity) {
//on switch theme activity is recreated
private boolean isThemeSwitchedFromHome = true;

//account for header
//will be helpful when them is switched
private int homeViewHeaderCount = 0;
Copy link
Member

Choose a reason for hiding this comment

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

default value for integer member variables will be 0, hence we can omit this initialisation.

@Sam1301
Copy link
Member

Sam1301 commented Mar 28, 2017

@vishwesh3 this looks great! 👍
Though I am still facing the load position bug on switching theme for a stream. Here's a short video https://drive.google.com/open?id=0B4jK5QX65b0ZbzZoR1BvVjd1M1E
Also noticed a typo in the last commit :).

@zulipbot
Copy link
Member

zulipbot commented Aug 9, 2017

Heads up @vishwesh3, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants