Opened 6 years ago

Last modified 4 years ago

#281 reviewing change

Define the acceptable ads opt-in dialog in XML

Reported by: fhd Assignee: Mailkov
Priority: P4 Milestone:
Module: Adblock-Plus-for-Android Keywords: goodfirstbug
Cc: Blocked By:
Blocking: Platform: Unknown
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://github.com/adblockplus/adblockplusandroid/pull/9

Description

Background

We're currently hard coding style properties, we shouldn't. See:
http://codereview.adblockplus.org/6196972490850304/diff/5707702298738688/src/org/adblockplus/android/Preferences.java#newcode160

What to change

We should have an XML resource for the acceptable ads opt-in message.

Change History (8)

comment:1 Changed 5 years ago by fhd

  • Keywords goodfirstbug added
  • Platform set to Unknown

comment:2 Changed 5 years ago by Mailkov

Hi , I would like to work on this change, I have already cloned the adblockplus for android from GitHub.

Last edited 5 years ago by Mailkov (previous) (diff)

comment:3 Changed 5 years ago by fhd

  • Owner set to Mailkov

Sure, sorry for the delay.

Please have a look at https://adblockplus.org/en/source#github for working with the GitHub mirrors.

comment:4 Changed 5 years ago by Mailkov

I would like to know the email that I send the contributor agreement,
For this issue I'm creating a common layout dialog for acceptable ads is right?

Last edited 5 years ago by Mailkov (previous) (diff)

comment:5 Changed 5 years ago by fhd

You can send the agreement to me, felix@….

This is just about the Acceptable Ads opt-in dialog, pretty small one. Still, we should define it via XML rather than hard code it.

comment:6 Changed 5 years ago by Mailkov

This is the current code

private void showNotificationDialog(final String title, String message, String url)
  {
    url = TextUtils.htmlEncode(url);
    message = TextUtils.htmlEncode(message)
        .replaceAll("&lt;a&gt;(.*?)&lt;/a&gt;", "<a href=\"" + url + "\">$1</a>");
    final TextView messageView = new TextView(this);
    messageView.setText(Html.fromHtml(message));  
    messageView.setMovementMethod(LinkMovementMethod.getInstance());
    final int padding = 10;
    messageView.setPadding(padding, padding, padding, padding);
    new AlertDialog.Builder(this).setTitle(title)
        .setView(messageView)
        .setIcon(android.R.drawable.ic_dialog_info)
        .setPositiveButton(R.string.ok, null).create().show();
      
  }

So , I thought I would create a XML layout to add a common AlertDialog.

Example partial code :

<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
    android:layout_width="fill_parent"
    android:layout_height="fill_parent"
    android:orientation="vertical" >

    <TextView
        android:id="@+id/acceptable_ads_message"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:padding="10dp" />

</LinearLayout>

Is it right?
Or Want you a simple xml resource for 10 or other?

If not, Can You show me the rows code currently hard coding style.

Thank's for help

Last edited 5 years ago by Mailkov (previous) (diff)

comment:7 Changed 5 years ago by Mailkov

pull request: https://github.com/adblockplus/adblockplusandroid/pull/9

Can you review this ? Thanks.

comment:8 Changed 4 years ago by fhd

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing
  • Tester set to Unknown

Seems we forgot to change the state here. It's a bit crazy for us right now in Adblock Browser, I hope we'll get around to have a look soon.

Note: See TracTickets for help on using tickets.