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

Button design improvement for issue Dialog #62

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

Conversation

JubrilEdu
Copy link

@JubrilEdu JubrilEdu commented Oct 7, 2017

Makes the dialog buttons in the dialog look like buttons and not links

Changes proposed in this pull request:

Implemented a custom alertdialog with view as custom_dialog.xml and border.xml for button borders. Increase gradle version to 2.3.3 from 2.3.1

@naushad-madakiya naushad-madakiya changed the title Made adjustments to the dialog buttons as requested in issue #12 Button design improvement for Issue Dialog Oct 7, 2017
@naushad-madakiya naushad-madakiya changed the title Button design improvement for Issue Dialog Button design improvement for issue Dialog Oct 7, 2017
android:layout_toLeftOf="@id/ok"
android:background="@drawable/border"
android:elevation="5dp"
android:text="EDIT" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the existing strings from the string resource.

android:layout_marginTop="30dp"
android:background="@drawable/border"
android:elevation="5dp"
android:text="OK" />
Copy link
Contributor

Choose a reason for hiding this comment

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

Use text from the string resource.

android:layout_marginTop="30dp"
android:background="@drawable/border"
android:elevation="5dp"
android:text="DELETE" />
Copy link
Contributor

Choose a reason for hiding this comment

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

use text from the string resource

@JubrilEdu
Copy link
Author

Done @naushad-madakiya

@JubrilEdu
Copy link
Author

@naushad-madakiya @manparvesh Could you kindly act on this?

Copy link
Contributor

@naushad-madakiya naushad-madakiya left a comment

Choose a reason for hiding this comment

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

AlertDialog should be dismissed when Delete operation is performed.

  • Remove auto-generated files.
  • Use better naming conventions.
  • Change custom_dialog.xml to relevant name.

final AlertDialog.Builder builder = new AlertDialog.Builder(MainActivity.getInstance());
View view = LayoutInflater.from(MainActivity.getInstance()).inflate(R.layout.custom_dialog, null);
builder.setView(view);
Button editBitton = (Button) view.findViewById(R.id.edit);
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix typo with editBitton

.setNegativeButtonColorRes(R.color.md_pink_a200)
.show();

final AlertDialog.Builder builder = new AlertDialog.Builder(MainActivity.getInstance());
Copy link
Contributor

Choose a reason for hiding this comment

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

  • AlertDialog.Builder is not required to make final
  • Should use better-naming convention i.e. editDialogBuilder instead of builder only.

build.gradle Outdated
@@ -8,7 +8,7 @@ buildscript {
}
}
dependencies {
classpath 'com.android.tools.build:gradle:2.3.1'
classpath 'com.android.tools.build:gradle:2.3.3'
Copy link
Contributor

Choose a reason for hiding this comment

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

Should not change configuration if not required.

@naushad-madakiya
Copy link
Contributor

naushad-madakiya commented Oct 17, 2017

@djubreel can you remove the autogenerated files from .idea directory which you have added?

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

Successfully merging this pull request may close these issues.

None yet

3 participants