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

add rate button #2036

Merged
merged 1 commit into from
Jan 18, 2020
Merged

add rate button #2036

merged 1 commit into from
Jan 18, 2020

Conversation

makoteq
Copy link
Contributor

@makoteq makoteq commented Dec 20, 2019 •
edited
Loading

fix for #2035

Changes: I added button in navbar with link to google play
Checklist: [Please tick following check boxes with [x] if the respective task is completed]

  • I have used resources from strings.xml, dimens.xml and colors.xml without hard-coding them
  • No modifications done at the end of resource files strings.xml, dimens.xml or colors.xml
  • I have reformatted code in every file included in this PR [CTRL+ALT+L]
  • My code does not contain any extra lines or extra spaces
  • I have requested reviews from other members

APK for testing:
app-fdroid-debug.zip

@makoteq
Copy link
Contributor Author

makoteq commented Dec 22, 2019

@CloudyPadmal Can you review, please

@cweitat
Copy link
Contributor

cweitat commented Jan 5, 2020 •
edited
Loading

please show a GIF of how it looks like and the flow

@makoteq
Copy link
Contributor Author

makoteq commented Jan 5, 2020

GIF-200105_190325
@cweitat

@cweitat
Copy link
Contributor

cweitat commented Jan 5, 2020

Nice. Looks good. Can the Rate app be above share app and also capital A for the app word?
Icon should be a star instead of message icon as well

@makoteq
Copy link
Contributor Author

makoteq commented Jan 5, 2020

app-fdroid-debug.zip
I changed icon and move rate button above share button @cweitat

@makoteq
Copy link
Contributor Author

makoteq commented Jan 5, 2020 •
edited
Loading

2020-01-05 (2)
@cweitat

cweitat
cweitat requested changes Jan 6, 2020
View reviewed changes
app/src/main/res/values/strings.xml Outdated
@@ -1037,5 +1037,6 @@
<string name="no_location_specified">Location not specified</string>
<string name="compass_default_0">0</string>
<string name="unknown">Unknown</string>
<string name="rateapp">Rate app</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be consistent here. capital A for the app. same as Share App

@makoteq
Copy link
Contributor Author

makoteq commented Jan 6, 2020

@cweitat Done

@cweitat cweitat requested a review from mariobehling January 6, 2020 11:55
@makoteq
Copy link
Contributor Author

makoteq commented Jan 9, 2020

@mariobehling Can you review please?

@fcartegnie fcartegnie self-requested a review January 9, 2020 18:40
fcartegnie
fcartegnie approved these changes Jan 9, 2020
View reviewed changes
Copy link

@fcartegnie fcartegnie left a comment

Choose a reason for hiding this comment

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

Minor change

@fcartegnie fcartegnie self-requested a review January 9, 2020 18:42
fcartegnie
fcartegnie requested changes Jan 9, 2020
View reviewed changes
Copy link

@fcartegnie fcartegnie left a comment

Choose a reason for hiding this comment

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

remove unrelated formatting changes

app/src/main/java/io/pslab/activity/MainActivity.java Outdated
@@ -484,21 +490,21 @@ public void onReceive(Context context, Intent intent) {
initialisationDialog.dismiss();
invalidateOptionsMenu();
CustomSnackBar.showSnackBar(findViewById(android.R.id.content),
getString(R.string.device_connected_successfully),null,null, Snackbar.LENGTH_SHORT);
getString(R.string.device_connected_successfully), null, null, Snackbar.LENGTH_SHORT);
Copy link

Choose a reason for hiding this comment

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

this is not related to the commit

app/src/main/java/io/pslab/activity/MainActivity.java Outdated
if (navItemIndex == 0) {
getSupportFragmentManager().beginTransaction().replace(R.id.frame, InstrumentsFragment.newInstance()).commit();
} else if (navItemIndex == 1) {
getSupportFragmentManager().beginTransaction().replace(R.id.frame, HomeFragment.newInstance(true, true)).commitAllowingStateLoss();
} else {
CustomSnackBar.showSnackBar(findViewById(android.R.id.content),
getString(R.string.device_connected_successfully),null,null, Snackbar.LENGTH_SHORT);
getString(R.string.device_connected_successfully), null, null, Snackbar.LENGTH_SHORT);
Copy link

Choose a reason for hiding this comment

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

ditto

app/src/main/java/io/pslab/activity/MainActivity.java Outdated
}
}
} else {
initialisationDialog.dismiss();
Log.d(TAG, "permission denied for device " + device);
CustomSnackBar.showSnackBar(findViewById(android.R.id.content),
getString(R.string.device_not_found),null,null, Snackbar.LENGTH_SHORT);
getString(R.string.device_not_found), null, null, Snackbar.LENGTH_SHORT);
Copy link

Choose a reason for hiding this comment

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

ditto

@makoteq
Copy link
Contributor Author

makoteq commented Jan 9, 2020

@fcartegnie Done!

@fcartegnie
Copy link

No. You made it worse.
You must not include unrelated changes.

@makoteq
Copy link
Contributor Author

makoteq commented Jan 9, 2020

@fcartegnie I dont really understand, how i can do this?

@makoteq
Copy link
Contributor Author

makoteq commented Jan 10, 2020

@fcartegnie Now these changes are not include

@fcartegnie
Copy link

@fcartegnie I dont really understand, how i can do this?

Learn to squash/fixup commits.
This is incremental changes, not text editor. You can't have do and undo commits.

@makoteq makoteq closed this Jan 11, 2020
@makoteq makoteq reopened this Jan 11, 2020
fcartegnie
fcartegnie requested changes Jan 11, 2020
View reviewed changes
app/src/main/res/values-ru/strings.xml Outdated
@@ -1,21 +1,21 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<string name="app_name">PSLab</string>
<string name="openDrawer">открыть</string>
<string name="closeDrawer">закрыть</string>
<string name="openDrawer">open_drawer</string>
Copy link

Choose a reason for hiding this comment

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

why did you revert the translations ?

@makoteq
Copy link
Contributor Author

makoteq commented Jan 11, 2020

@fcartegnie Sorry for that complications, what do you think now?

@makoteq
Copy link
Contributor Author

makoteq commented Jan 13, 2020

@fcartegnie What about now?

@fcartegnie fcartegnie requested a review from cweitat January 14, 2020 12:14
@cweitat cweitat requested a review from fcartegnie January 14, 2020 16:57
@fcartegnie
Copy link

I can't approve try & fix commits. Make things clean first

@makoteq
Copy link
Contributor Author

makoteq commented Jan 16, 2020

@fcartegnie I squashed my commits, i think its ready to merge, also sorry for problems

@CloudyPadmal CloudyPadmal added the Feature New addition to the existing app label Jan 17, 2020
@CloudyPadmal CloudyPadmal merged commit e028840 into fossasia:development Jan 18, 2020
@CloudyPadmal CloudyPadmal mentioned this pull request Jan 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cweitat cweitat cweitat approved these changes

@CloudyPadmal CloudyPadmal CloudyPadmal approved these changes

@mariobehling mariobehling Awaiting requested review from mariobehling

@fcartegnie fcartegnie Awaiting requested review from fcartegnie

Assignees
No one assigned
Labels
Feature New addition to the existing app
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@makoteq @cweitat @fcartegnie @CloudyPadmal