Add tutorial sliders for first time launch of app #466
Conversation
There was a problem hiding this comment.
Instead of creating 4 different layout's you can create one base fragment class and it's corresponding base layout. Then create 4 different instance of this base fragment with different image and description. This might be helpful http://stackoverflow.com/questions/27371565/use-same-fragment-in-viewpager-but-fragment-will-have-different-layout-each-time
|
@vishwesh3 yeah i tried same the first time but all layouts have different sizes and of imagview and paddings so it is difficult to have same dimensions in all the 4 layouts |
|
@saketkumar95 How about using the App intro Library Material AppIntro? |
|
@nitish1211 The library looks awesome but we are using simple intro slides and i just used 1 class and 4 layouts so it doesn't make sense for using library for that only. We already have lots of library and apk size is already high right now. :) |
|
Anyone Working on this ? I want to work on ViewPager and add Multiple instances of Fragment instead of 4 Different Layouts. |
|
@saketkumar95 thanks for working on this! 👍 One issue: I noticed the shadow under the toolbar and menu in zulipActivity are lost. |
| @@ -0,0 +1,54 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| <RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android" | |||
There was a problem hiding this comment.
okay so the tutorial_slide 1 to 4 are very similar, except the fact that the views point to different resources.
I suggest we just use a single .xml file and customise it for different slides. This would reduce the number of lines of code significantly.
| <Button | ||
| android:id="@+id/btn_next" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" |
There was a problem hiding this comment.
we can abstract out the style of buttons here into styles.xml
| android:gravity="center" | ||
| android:orientation="horizontal"> | ||
|
|
||
|
|
There was a problem hiding this comment.
there are some extra newlines in other xml files as well
| <LinearLayout | ||
| android:id="@+id/layoutDots" | ||
| android:layout_width="match_parent" | ||
| android:layout_height="30dp" |
There was a problem hiding this comment.
how about saving dimensions in dimen.xml?
| } | ||
|
|
||
| public void setFirstTimeLaunch(boolean isFirstTime) { | ||
| editor.putBoolean(IS_FIRST_TIME_LAUNCH, isFirstTime); |
There was a problem hiding this comment.
how about saving the version of app instead of a boolean; shared pref will remain same across upgrades
| private static final String IS_FIRST_TIME_LAUNCH = "IsFirstTimeLaunch"; | ||
| SharedPreferences pref; | ||
| SharedPreferences.Editor editor; | ||
| Context _context; |
There was a problem hiding this comment.
Non-public, non-static field names start with m.
| @@ -32,6 +32,8 @@ | |||
| android:allowBackup="false" | |||
| android:icon="@drawable/ic_launcher" | |||
| android:label="@string/app_name" | |||
| android:hardwareAccelerated="false" | |||
| android:largeHeap="true" | |||
There was a problem hiding this comment.
what is the story behind this change?
There was a problem hiding this comment.
@Sam1301 There was an issue outOfMemory error with loading the images. So i added largeheap to optimize using memory. :)
|
|
||
| <item | ||
| android:id="@+id/helpMe" | ||
| android:title="Help" /> |
There was a problem hiding this comment.
hardcoded string; also maybe calling it "Tutorial" or "Take a tour" would be better?
|
@saketkumar95 I would want to merge this, can you fix the comments posted by saumya? |
|
@kunall17 Yeah i will do this in 1-2 days, currently busy in my lab exams. :/ |
|
@saketkumar95, your pull request has developed a merge conflict! Please review the most recent commit (f791888) for conflicting changes and resolve your pull request's merge conflicts. |
Summary of changes
Fixes https://github.com/zulip/zulip-android/issues/161
Please make sure these boxes are checked before submitting your pull request - thanks! Guide
Code is formatted.
Run the lint tests with
./gradlew lintDebugand make sure BUILD is SUCCESSFULIf new feature is implemeted then it is compatible with Night theme too.
Commit messages are well-written.