"Skip Tutorial" button moved to parent view so that it does not animate by swipes. #1945

mrahimygk · 2018-10-20T16:06:35Z

Title

"Skip Tutorial" button moved to parent view so that it does not animate by swipes.

Description

I noticed that the button moves by swipes, so I moved it to the parent view.
And I saw that the tutorial is not showing after I started the app again (and I didn't follow the tut, so I needed it again) , I moved the code that says "it is not the first run from now on" to the appropriate activity.

Tests performed

Tested on:

API level 19 : Samsung Grand Prime.
android 8 : Huawei mate 10 lite

Screenshots showing what changed

http://leyth.persiangig.com/github/Screenshot_2018-10-20-19-29-26.png

Now it starts welcome screen after not finishing the pager. Moved "Skip Tutorial" button here so it does not animate by swipe.

Putting "Skip Tutorial" button here so that it does not animate by swipes.

Removing the set of "first run " flag from here. we set it after the buttons press or on WelcomeActivity's finish()

removing "skip tut" button from here.

Removing "Skip Tutorial" button from here so that it does not animate by swipes.

the "welcomyesButton" is removed from the child views in pager, so it is optional now.

codecov-io · 2018-10-20T16:31:46Z

Codecov Report

Merging #1945 into master will increase coverage by 1.16%.
The diff coverage is 0%.

@@            Coverage Diff            @@
##           master   #1945      +/-   ##
=========================================
+ Coverage    4.36%   5.53%   +1.16%     
=========================================
  Files         211     233      +22     
  Lines       10643   11582     +939     
  Branches      954    1075     +121     
=========================================
+ Hits          465     641     +176     
- Misses      10143   10888     +745     
- Partials       35      53      +18

Impacted Files	Coverage Δ
...n/java/fr/free/nrw/commons/auth/LoginActivity.java	`0% <ø> (ø)`	⬆️
.../java/fr/free/nrw/commons/WelcomePagerAdapter.java	`0% <0%> (ø)`	⬆️
...main/java/fr/free/nrw/commons/WelcomeActivity.java	`0% <0%> (ø)`	⬆️
...s/ui/LongTitlePreferences/LongTitlePreference.java	`18.18% <0%> (-36.37%)`	⬇️
.../free/nrw/commons/di/CommonsApplicationModule.java	`27.5% <0%> (-18.34%)`	⬇️
app/src/main/java/fr/free/nrw/commons/Utils.java	`24.39% <0%> (-8.95%)`	⬇️
...a/fr/free/nrw/commons/utils/ContributionUtils.java	`7.14% <0%> (-4.98%)`	⬇️
...n/java/fr/free/nrw/commons/CommonsApplication.java	`41.77% <0%> (-2.14%)`	⬇️
...free/nrw/commons/theme/NavigationBaseActivity.java	`24.26% <0%> (-1.12%)`	⬇️
...ain/java/fr/free/nrw/commons/upload/FileUtils.java	`2.13% <0%> (-1.09%)`	⬇️
... and 92 more

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 7b7d17a...f224c50. Read the comment docs.

albendz · 2018-10-22T23:52:18Z

Will test.

albendz · 2018-10-23T02:19:52Z

Sorry, can't test. Failing builds from PR and master. Will need to figure this out separately.

maskaravivek · 2018-10-28T08:24:11Z

The PR works well for me but I just noticed that on a fresh install the WelcomeActivity isn't launched(both on master and this PR branch). The PR fixes the issue where "Skip tutorial" used to animate on swiping the page but we need to check why WelcomeActivity isn't being shown on fresh installs.

domdomegg

Thanks for the PR!

Just a couple of comments:

I'd add a couple of missing JavaDocs - I think it would be useful to explain rationale behind onBackPressed (would basically be same as comment on L98) and finishTutorial
Not required for this PR, but it might be an idea to rename the id for the skip button. I believe it used to be a yes/no choice but now as it's just a skip button it should have a name to reflect that.

app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java

app/src/main/res/layout/activity_welcome.xml

domdomegg · 2018-12-11T22:12:43Z

@m-rahimy You've made good changes - are you able to make the changes I've requested?

maskaravivek · 2018-12-12T05:07:27Z

@domdomegg I think @m-rahimy is away and if the functionality added by the PR is fine, we can probably merge the PR and open separate issues for the comments that you have added. :)

domdomegg · 2018-12-12T10:30:25Z

Digging deeper, we also probably want the same behavior on landscape so those layouts need to be updated too. Have added Javadoc, and happy @maskaravivek to merge.

maskaravivek · 2018-12-12T11:02:35Z

@domdomegg Can you open an issue for landscape mode?

domdomegg · 2018-12-12T11:08:59Z

@domdomegg Can you open an issue for landscape mode?

#2103

domdomegg

Doing some more testing this crashes the beta version as it looks for the welcomeYesButton in the page layout which doesn't exist, causing a NPE when it tries to set visibility. Will try to fix myself.

domdomegg · 2018-12-16T14:34:46Z

Will need someone else to merge given that I've worked on this.

Tests performed

All automated tests pass

2.8.5-debug-pr-1945~f224c50a (beta) on Samsung Galaxy S6 at API level 25
2.8.5-debug-pr-1945~f224c50a (prod) on Samsung Galaxy S6 at API level 25
2.8.5-debug-pr-1945~f224c50a (beta) on Galaxy Nexus (simulator) at API level 28
2.8.5-debug-pr-1945~f224c50a (prod) on Galaxy Nexus (simulator) at API level 28

Things that work:

Skip tutorial button on first page (beta only)
Skip tutorial button on last page (beta only)
Skip tutorial button not present (prod only)
"YES!" button on last page
More information button on last page
Can't back out of tutorial on first run
Press back out of tutorial if not first run

neslihanturan · 2018-12-17T09:40:57Z

Thanks both @m-rahimy and @domdomegg ! This works as expected.

mrahimygk added 8 commits October 20, 2018 17:43

Update WelcomeActivity.java

0806548

Now it starts welcome screen after not finishing the pager. Moved "Skip Tutorial" button here so it does not animate by swipe.

Update activity_welcome.xml

8d50621

Putting "Skip Tutorial" button here so that it does not animate by swipes.

Update LoginActivity.java

2e745fa

Removing the set of "first run " flag from here. we set it after the buttons press or on WelcomeActivity's finish()

Update welcome_do_upload.xml

ab899e2

removing "skip tut" button from here.

Update welcome_dont_upload.xml

76cd049

Removing "Skip Tutorial" button from here so that it does not animate by swipes.

Update welcome_image_details.xml

f504f63

Removing "Skip Tutorial" button from here so that it does not animate by swipes.

Update welcome_wikipedia.xml

40b810b

Removing "Skip Tutorial" button from here so that it does not animate by swipes.

Update WelcomePagerAdapter.java

5a4e172

the "welcomyesButton" is removed from the child views in pager, so it is optional now.

maskaravivek approved these changes Oct 28, 2018

View reviewed changes

domdomegg requested changes Nov 2, 2018

View reviewed changes

app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java Show resolved Hide resolved

app/src/main/java/fr/free/nrw/commons/WelcomeActivity.java Outdated Show resolved Hide resolved

app/src/main/res/layout/activity_welcome.xml Outdated Show resolved Hide resolved

Merge branch 'master' into master

40f74a9

commons-app deleted a comment Nov 4, 2018

Add JavaDoc to WelcomeActivity.onBackPressed()

f395ff9

domdomegg approved these changes Dec 12, 2018

View reviewed changes

domdomegg mentioned this pull request Dec 12, 2018

Skip tutorial button moves with screen in landscape mode #2103

Closed

domdomegg requested changes Dec 16, 2018

View reviewed changes

domdomegg added 2 commits December 16, 2018 14:15

Fix commons-app#2103: Remove welcomeYesButton from landscape layout

17e8767

Refactor WelcomePagerAdapter

f224c50

domdomegg mentioned this pull request Dec 16, 2018

Added CDATA tag #1836

Merged

domdomegg approved these changes Dec 16, 2018

View reviewed changes

neslihanturan approved these changes Dec 17, 2018

View reviewed changes

neslihanturan merged commit f79456e into commons-app:master Dec 17, 2018

"Skip Tutorial" button moved to parent view so that it does not animate by swipes. #1945

"Skip Tutorial" button moved to parent view so that it does not animate by swipes. #1945

Uh oh!

Conversation

mrahimygk commented Oct 20, 2018

Title

Description

Tests performed

Screenshots showing what changed

Uh oh!

codecov-io commented Oct 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

albendz commented Oct 22, 2018

Uh oh!

albendz commented Oct 23, 2018

Uh oh!

maskaravivek commented Oct 28, 2018

Uh oh!

domdomegg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

domdomegg commented Dec 11, 2018

Uh oh!

maskaravivek commented Dec 12, 2018

Uh oh!

domdomegg commented Dec 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maskaravivek commented Dec 12, 2018

Uh oh!

domdomegg commented Dec 12, 2018

Uh oh!

domdomegg left a comment

Choose a reason for hiding this comment

Uh oh!

domdomegg commented Dec 16, 2018

Uh oh!

neslihanturan commented Dec 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-io commented Oct 20, 2018 •

edited

Loading

domdomegg commented Dec 12, 2018 •

edited

Loading