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.

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

Fix: Replace usage dialogue box with New Usage Activity #65

Merged
merged 1 commit into from Mar 2, 2018

Conversation

Hiteshgautam01
Copy link
Contributor

@Hiteshgautam01 Hiteshgautam01 commented Mar 1, 2018

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

  • Apply the AndroidStyle.xml style template to your code in Android Studio.

  • Run the checks with ./gradlew check to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

@Hiteshgautam01
Copy link
Contributor Author

Usage Activity UI

screen shot 2018-02-27 at 10 16 01 pm

Copy link
Contributor

@saketkumar saketkumar left a comment

Choose a reason for hiding this comment

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

@Hiteshgautam01 Great work! 👍Thanks for the PR. I have added a comment.

@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.activity_usage2);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Hiteshgautam01 layout name should be activity_usage only. Why usage2?

dialog.setTitle(getString(R.string.title_nav_usage));
dialog.setContentView(R.layout.usage_layout);
dialog.show();
Intent i = new Intent(DashboardActivity.this, UsageActivity.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

use intent instead of i

android:layout_height="match_parent"
android:orientation="vertical">

<android.support.v7.widget.Toolbar
Copy link
Contributor

Choose a reason for hiding this comment

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

@Hiteshgautam01 This is not required here. You just need to include appbar.xml layout file here. appbar.xml file already have Toolbar defined.

@sagar15795
Copy link
Member

@saketkumar95
Thanks for the review.

@Hiteshgautam01 Hiteshgautam01 force-pushed the FixUsage branch 2 times, most recently from c263a83 to f48b2ab Compare March 2, 2018 02:18
Copy link
Contributor

@saketkumar saketkumar left a comment

Choose a reason for hiding this comment

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

@Hiteshgautam01 Everything looks good now. Just fix that checkstyle issue. 👍

dialog.setTitle(getString(R.string.title_nav_usage));
dialog.setContentView(R.layout.usage_layout);
dialog.show();
Intent intent = new Intent(DashboardActivity.this, UsageActivity.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Hiteshgautam01 Checkstyle rule is failing here.

Copy link
Contributor

@saketkumar saketkumar left a comment

Choose a reason for hiding this comment

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

Good work! @Hiteshgautam01 👍
@sagar15795 LGTM!

@sagar15795
Copy link
Member

@saketkumar95 Thanks for reviewing it.

@asfgit asfgit merged commit 44c6ed3 into apache:master Mar 2, 2018
asfgit pushed a commit that referenced this pull request Mar 2, 2018
@sagar15795
Copy link
Member

@Hiteshgautam01 Thanks for the UI improvement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants