Initial UI structure for autofill dialog

chrome/android/java/src/org/chromium/chrome/browser/autofill/AutofillDialogContentView.java

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+package org.chromium.chrome.browser.autofill;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import android.annotation.SuppressLint;
+import android.content.Context;
+import android.text.style.UpdateLayout;
+import android.util.AttributeSet;
+import android.view.View;
+import android.view.ViewGroup;
+import android.view.animation.Animation;
+import android.view.animation.AnimationSet;
+import android.view.animation.ScaleAnimation;
+import android.view.animation.TranslateAnimation;
+import android.widget.AdapterView;
+import android.widget.ArrayAdapter;
+import android.widget.FrameLayout;
+import android.widget.ImageView;
+import android.widget.Spinner;
+import android.widget.TextView;
+import android.widget.AdapterView.OnItemSelectedListener;
+
+import org.chromium.chrome.R;
+
+/**
+ * This is the parent layout that contains the layouts for different states of
+ * autofill dialog. In principle it shouldn't contain any logic related with the
+ * actual workflow, but rather respond to any UI update messages coming to it
+ * from the AutofillDialog.
+ */
+public class AutofillDialogContentView extends FrameLayout {
+    private static int ANIMATION_DURATION_MS = 1000;
+    private static int PLACEHOLDER_ANIMATION_DISTANCE = 500;

[Ted C, 2013/03/06 01:41:56]

what unit is this distance in?  Px?  Dp?

[Yusuf, 2013/03/06 19:50:00]

Done.

+    private static final List<AddressItem> mConstantShippingItems =
+            new ArrayList<AddressItem>();
+    private static final List<CreditCardItem> mConstantBillingItems =

[Ted C, 2013/03/06 01:41:56]

why are these static?

[Yusuf, 2013/03/06 19:50:00]

Done.

+            new ArrayList<CreditCardItem>();
+
+    private ArrayList<View> mTopViewGroup;
+    private ArrayList<View> mMidViewGroup;
+    private ArrayList<View> mBottomViewGroup;
+    private Spinner mCreditCardSpinner;
+    private Spinner mAddressSpinner;
+    private View mSteadyLayout;
+    private View mEditShippingLayout;
+    private View mEditBillingLayout;
+
+    public AutofillDialogContentView(Context context, AttributeSet attrs) {
+        super(context, attrs);
+        if (mConstantShippingItems.size() == 0) {

[Ted C, 2013/03/06 01:41:56]

Does list have isEmpty()?

[Yusuf, 2013/03/06 19:50:00]

Done.

+            mConstantShippingItems.add(new AddressItem(
+                    getResources().getString(R.string.autofill_new_shipping), ""));
+            mConstantShippingItems.add(new AddressItem(
+                    getResources().getString(R.string.autofill_edit_shipping), ""));
+        }
+        if (mConstantBillingItems.size() == 0) {
+            mConstantBillingItems.add(new CreditCardItem(0,
+                    getResources().getString(R.string.autofill_new_billing), ""));
+            mConstantBillingItems.add(new CreditCardItem(0,
+                    getResources().getString(R.string.autofill_edit_billing), ""));
+        }
+        mTopViewGroup = new ArrayList<View>();

[Ted C, 2013/03/06 01:41:56]

these can probably be final since they're not reset.

[Yusuf, 2013/03/06 19:50:00]

Done.

+        mMidViewGroup = new ArrayList<View>();
+        mBottomViewGroup = new ArrayList<View>();
+    }
+
+    @Override
+    protected void onFinishInflate () {
+        mSteadyLayout = findViewById(R.id.general_layout);
+        mEditBillingLayout = findViewById(R.id.editing_layout_billing);
+        mEditShippingLayout = findViewById(R.id.editing_layout_shipping);
+
+        mCreditCardSpinner = (Spinner)findViewById(R.id.cc_spinner);

[Ted C, 2013/03/06 01:41:56]

space after case (double check all your casts) 

[Yusuf, 2013/03/06 19:50:00]

Done.

+        List<CreditCardItem> cards = new ArrayList<CreditCardItem>();
+        //TODO(yusufo): Remove all placeholders here and also in related layout xml files.
+        cards.add(new CreditCardItem(R.drawable.visa,

[Ted C, 2013/03/06 01:41:56]

again, move the placeholders to a function that can be replaced with working
functionality easily.

[Yusuf, 2013/03/06 19:50:00]

Done.

+                "XXXX-XXXX-XXXX-1000", "Additional info required"));
+        cards.add(new CreditCardItem(R.drawable.master_card, "XXXX-XXXX-XXXX-1000", ""));
+        cards.addAll(mConstantBillingItems);
+        mCreditCardSpinner.setAdapter(new CreditCardAdapter(getContext(), cards));
+        mAddressSpinner = (Spinner)findViewById(R.id.address_spinner);
+        List<AddressItem> addresses = new ArrayList<AddressItem>();
+        addresses.add(new AddressItem("Place Holder", "1600 Amphitheatre Pkwy"));
+        addresses.addAll(mConstantShippingItems);
+        mAddressSpinner.setAdapter(new AddressAdapter(getContext(), addresses));
+
+        setViewGroups();
+        changeLayoutTo(AutofillDialog.LAYOUT_STEADY);
+    }
+
+    @Override
+    protected void onMeasure (int widthMeasureSpec, int heightMeasureSpec) {
+        super.onMeasure(widthMeasureSpec, heightMeasureSpec);
+        mCreditCardSpinner.setDropDownWidth(mCreditCardSpinner.getMeasuredWidth());
+        mCreditCardSpinner.setDropDownVerticalOffset(-1 * mCreditCardSpinner.getMeasuredHeight());

[Ted C, 2013/03/06 01:41:56]

you can just do:

-mCreditCardSpinner.getMeasuredHeight()

same below

[Yusuf, 2013/03/06 19:50:00]

Done.

+        mAddressSpinner.setDropDownWidth(mAddressSpinner.getMeasuredWidth());
+        mAddressSpinner.setDropDownVerticalOffset(-1 * mAddressSpinner.getMeasuredHeight());
+    }
+
+    /**
+     * Set the listener for all the dropdown members in the layout.
+     * @param listener The listener object to attach to the dropdowns.
+     */
+    public void setOnItemSelectedListener(OnItemSelectedListener listener) {
+        mCreditCardSpinner.setOnItemSelectedListener(listener);
+        mAddressSpinner.setOnItemSelectedListener(listener);
+    }
+
+    /**
+     * @param spinner The dropdown that was selected by the user.
+     * @param position The position for the selected item in the dropdown.
+     * @return Whether the selection should cause a layout state change.
+     */
+    public boolean selectionShouldChangeLayout(AdapterView<?> spinner, int position) {
+        int numConstantItems = spinner.getId() == R.id.cc_spinner ?
+                mConstantBillingItems.size() : mConstantShippingItems.size();
+        return position >= spinner.getCount() - numConstantItems;
+    }
+
+    /**
+     * Transitions the layout shown to a given layout.
+     * @param mode The layout mode to transition to.
+     */
+    public void changeLayoutTo(int mode) {

[Ted C, 2013/03/06 01:41:56]

this function is too long, look into splitting it up.

+        mEditBillingLayout.setVisibility(GONE);

[Ted C, 2013/03/06 01:41:56]

if you're already in the current mode, should you exit early?

[Yusuf, 2013/03/06 19:50:00]

Done.

+        mEditShippingLayout.setVisibility(GONE);
+        mSteadyLayout.setVisibility(VISIBLE);
+        if (mode == AutofillDialog.LAYOUT_STEADY) return;
+
+        View centerView;
+        View centeredLayout;
+
+        TranslateAnimation toTopAnimation = new TranslateAnimation(
+                0, 0, 0, -PLACEHOLDER_ANIMATION_DISTANCE);
+        toTopAnimation.setDuration(ANIMATION_DURATION_MS);
+        TranslateAnimation toBottomAnimation = new TranslateAnimation(
+                0, 0, 0, PLACEHOLDER_ANIMATION_DISTANCE);
+        toBottomAnimation.setDuration(ANIMATION_DURATION_MS);
+        TranslateAnimation midViewGroupAnimation;
+        if (mode == AutofillDialog.LAYOUT_EDITING_BILLING) {
+            centerView = mCreditCardSpinner;
+            centeredLayout = mEditBillingLayout;
+            midViewGroupAnimation = toBottomAnimation;
+        } else {
+            centerView = mAddressSpinner;
+            centeredLayout = mEditShippingLayout;
+            midViewGroupAnimation = toTopAnimation;
+        }
+        float yOffset = centerView.getY() - centerView.getHeight() / 2;
+        TranslateAnimation toLocationAnimation = new TranslateAnimation(0, 0, 0, 0);
+        toLocationAnimation.setDuration(ANIMATION_DURATION_MS);
+        ScaleAnimation scaleAnimation;
+        scaleAnimation = new ScaleAnimation(1, 1, 0, 1,
+                Animation.RELATIVE_TO_SELF, 0, Animation.RELATIVE_TO_SELF, 0.5f);
+        scaleAnimation.setDuration(ANIMATION_DURATION_MS);
+        ScaleAnimation scaleDownAnimation;
+        scaleDownAnimation = new ScaleAnimation(1, 1, 1, 0,
+                Animation.RELATIVE_TO_SELF, 0, Animation.RELATIVE_TO_SELF, 0.5f);
+        scaleDownAnimation.setDuration(ANIMATION_DURATION_MS / 2);
+        scaleDownAnimation.setStartOffset(ANIMATION_DURATION_MS / 2);
+        scaleAnimation.setStartOffset(ANIMATION_DURATION_MS / 4);
+
+        AnimationSet appearAnimation = new AnimationSet(true);
+        appearAnimation.addAnimation(toLocationAnimation);
+        appearAnimation.addAnimation(scaleAnimation);
+
+        int i;

[Ted C, 2013/03/06 01:41:56]

I would declare i in each of the loops to prevent value leakage unintentionally

[Yusuf, 2013/03/06 19:50:00]

Done.

+        for (i = 0; i < mTopViewGroup.size(); i++) {
+            mTopViewGroup.get(i).setAnimation(toTopAnimation);
+            mTopViewGroup.get(i).animate();
+        }
+        for (i = 0; i < mMidViewGroup.size(); i++) {
+            mMidViewGroup.get(i).setAnimation(midViewGroupAnimation);
+            mMidViewGroup.get(i).animate();
+        }
+        for (i = 0; i < mBottomViewGroup.size(); i++) {
+            mBottomViewGroup.get(i).setAnimation(toBottomAnimation);
+            mBottomViewGroup.get(i).animate();
+        }
+
+        centeredLayout.setVisibility(VISIBLE);
+        centeredLayout.setAnimation(appearAnimation);
+        centeredLayout.animate();
+        mSteadyLayout.setAnimation(scaleDownAnimation);
+        mSteadyLayout.animate();
+        postDelayed(new Runnable() {

[Ted C, 2013/03/06 01:41:56]

should you be caching this runnable in case you need to remove it before it
executes?  (i.e. if you change layouts quickly).

[Yusuf, 2013/03/06 19:50:00]

Done.

+            @Override
+            public void run() {
+                mSteadyLayout.setVisibility(GONE);
+            }
+        }, ANIMATION_DURATION_MS);
+    }
+
+    private void setViewGroups() {

[Ted C, 2013/03/06 01:41:56]

I would move this up near the constructor.

[Yusuf, 2013/03/06 19:50:00]

Done.

+        mTopViewGroup.clear();
+        mTopViewGroup.add(findViewById(R.id.billing_title));
+        mTopViewGroup.add(findViewById(R.id.cc_spinner));
+
+        mMidViewGroup.clear();
+        mMidViewGroup.add(findViewById(R.id.shipping_title));
+        mMidViewGroup.add(findViewById(R.id.address_spinner));
+
+        mBottomViewGroup.clear();
+        mBottomViewGroup.add(findViewById(R.id.check_box));
+        mBottomViewGroup.add(findViewById(R.id.line_bottom));
+        mBottomViewGroup.add(findViewById(R.id.terms_info));
+    }
+
+    private static class AddressItem {
+        String mName;

[Ted C, 2013/03/06 01:41:56]

private final

[Yusuf, 2013/03/06 19:50:00]

Done.

+        String mAddress;
+        public AddressItem(String name, String address) {

[Ted C, 2013/03/06 01:41:56]

blank line above this

[Yusuf, 2013/03/06 19:50:00]

Done.

+            mName = name;
+            mAddress = address;
+        }
+    }
+
+    private static class AddressAdapter extends ArrayAdapter<AddressItem> {
+
+        public AddressAdapter(Context context, List<AddressItem> objects) {
+            super(context, R.layout.autofill_address_spinner_dropdown, objects);
+        }
+
+        @Override
+        public View getView(int position, View convertView, ViewGroup parent) {
+            return initView(position, convertView);
+        }
+
+        @Override
+        public View getDropDownView(int position, View convertView,
+                                    ViewGroup parent) {

[Ted C, 2013/03/06 01:41:56]

java indenting instead of c++

[Yusuf, 2013/03/06 19:50:00]

Done.

+            return initView(position, convertView);
+        }
+
+        private View initView(int position, View convertView) {
+            if(convertView == null)

[Ted C, 2013/03/06 01:41:56]

space after if, braces needed, and java indenting

[Yusuf, 2013/03/06 19:50:00]

Done.

+                convertView = View.inflate(getContext(),
+                                           R.layout.autofill_address_spinner_dropdown,
+                                           null);
+            TextView nameView = (TextView)convertView.findViewById(R.id.name);
+            TextView addressView = (TextView)convertView.findViewById(R.id.address);
+            if (nameView != null) nameView.setText(getItem(position).mName);

[Ted C, 2013/03/06 01:41:56]

I would cache getItem(position) before this.

[Yusuf, 2013/03/06 19:50:00]

Done.

+            if (addressView != null) addressView.setText(getItem(position).mAddress);
+            return convertView;
+        }
+    }
+
+    private static class CreditCardItem {

[Ted C, 2013/03/06 01:41:56]

Do these inner classes map to native counterparts?  might be worth documenting
that so we can figure out what the values relate to.

[Yusuf, 2013/03/06 19:50:00]

Done.

+        int mCardIconID;

[Ted C, 2013/03/06 01:41:56]

private final here too

[Yusuf, 2013/03/06 19:50:00]

No. See the added TODO to createAndAddPlaceholders(). They will be used just for
Android UI, so will be constructed on Java. (Maybe in here or in
AutofillDialog).


On 2013/03/06 01:41:56, Ted C wrote:

+        String mLastDigits;
+        String mInfo;
+        public CreditCardItem(int icon, String lastDigits, String info) {

[Ted C, 2013/03/06 01:41:56]

blank line above it.

[Yusuf, 2013/03/06 19:50:00]

Done.

+            mCardIconID = icon;
+            mLastDigits = lastDigits;
+            mInfo = info;
+        }
+    }
+
+    private static class CreditCardAdapter extends ArrayAdapter<CreditCardItem> {

[Ted C, 2013/03/06 01:41:56]

same comments as the above in regards to style.

[Yusuf, 2013/03/06 19:50:00]

Done.

+
+        public CreditCardAdapter(Context context, List<CreditCardItem> objects) {
+            super(context, R.layout.autofill_cc_spinner_dropdown, objects);
+        }
+
+        @Override
+        public View getView(int position, View convertView, ViewGroup parent) {
+            return initView(position, convertView);
+        }
+
+        @Override
+        public View getDropDownView(int position, View convertView,
+                                    ViewGroup parent) {
+            return initView(position, convertView);
+        }
+
+        private View initView(int position, View convertView) {
+            if(convertView == null)
+                convertView = View.inflate(getContext(),
+                                           R.layout.autofill_cc_spinner_dropdown,
+                                           null);
+            ImageView icon = (ImageView)convertView.findViewById(R.id.cc_icon);
+            TextView lastDigitsView = (TextView)convertView.findViewById(R.id.cc_number);
+            TextView infoView = (TextView)convertView.findViewById(R.id.cc_info);
+            if (icon != null) icon.setImageResource(getItem(position).mCardIconID);
+            if (lastDigitsView != null) lastDigitsView.setText(getItem(position).mLastDigits);
+            if (infoView != null) infoView.setText(getItem(position).mInfo);
+            return convertView;
+        }
+    }
+}