Fix a bug where custom cards are not refresh when removed

When the last custom card for a specific type is removed,
onContextualCardUpdated should receive the cardtype info so we can
remove it from main data set.

- Reverted onContextualCardUpdated method signature back to before
- Force ConditionContextualCardController to send an empty list if
  everything is removed.

* Note: the update logic is pretty complicated to handle
  add/update/remove all together. In the future we should consider
  spliting the removal logic to simplify this area.

Change-Id: Ied688deb693ec33e0017be02cf5c743a754a6e61
Fixes: 115572494
Test: visual
This commit is contained in:
Fan Zhang
2018-09-14 10:42:14 -07:00
parent 29aaf62410
commit 45fc707474
6 changed files with 33 additions and 33 deletions

View File

@@ -16,8 +16,6 @@
package com.android.settings.homepage; package com.android.settings.homepage;
import java.util.List;
/** /**
* Data controller for {@link ContextualCard}. * Data controller for {@link ContextualCard}.
*/ */
@@ -26,12 +24,6 @@ public interface ContextualCardController {
@ContextualCard.CardType @ContextualCard.CardType
int getCardType(); int getCardType();
/**
* When data is updated or changed, the new data should be passed to ContextualCardManager for
* list updating.
*/
void onDataUpdated(List<ContextualCard> cardList);
void onPrimaryClick(ContextualCard card); void onPrimaryClick(ContextualCard card);
void onActionClick(ContextualCard card); void onActionClick(ContextualCard card);

View File

@@ -18,8 +18,11 @@ package com.android.settings.homepage;
import static com.android.settings.homepage.CardContentLoader.CARD_CONTENT_LOADER_ID; import static com.android.settings.homepage.CardContentLoader.CARD_CONTENT_LOADER_ID;
import static java.util.stream.Collectors.groupingBy;
import android.content.Context; import android.content.Context;
import android.os.Bundle; import android.os.Bundle;
import android.util.ArrayMap;
import android.util.Log; import android.util.Log;
import android.widget.BaseAdapter; import android.widget.BaseAdapter;
@@ -33,6 +36,7 @@ import com.android.settingslib.core.lifecycle.LifecycleObserver;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.stream.Collectors; import java.util.stream.Collectors;
@@ -110,15 +114,12 @@ public class ContextualCardManager implements CardContentLoader.CardContentLoade
} }
@Override @Override
public void onContextualCardUpdated(List<ContextualCard> updateList) { public void onContextualCardUpdated(Map<Integer, List<ContextualCard>> updateList) {
//TODO(b/112245748): Should implement a DiffCallback. //TODO(b/112245748): Should implement a DiffCallback.
//Keep the old list for comparison. //Keep the old list for comparison.
final List<ContextualCard> prevCards = mContextualCards; final List<ContextualCard> prevCards = mContextualCards;
final Set<Integer> cardTypes = updateList final Set<Integer> cardTypes = updateList.keySet();
.stream()
.map(card -> card.getCardType())
.collect(Collectors.toSet());
//Remove the existing data that matches the certain cardType before inserting new data. //Remove the existing data that matches the certain cardType before inserting new data.
final List<ContextualCard> cardsToKeep = mContextualCards final List<ContextualCard> cardsToKeep = mContextualCards
.stream() .stream()
@@ -126,10 +127,10 @@ public class ContextualCardManager implements CardContentLoader.CardContentLoade
.collect(Collectors.toList()); .collect(Collectors.toList());
final List<ContextualCard> allCards = new ArrayList<>(); final List<ContextualCard> allCards = new ArrayList<>();
allCards.addAll(cardsToKeep); allCards.addAll(cardsToKeep);
allCards.addAll(updateList); allCards.addAll(
updateList.values().stream().flatMap(List::stream).collect(Collectors.toList()));
sortCards(allCards); sortCards(allCards);
//replace with the new data //replace with the new data
mContextualCards.clear(); mContextualCards.clear();
mContextualCards.addAll(allCards); mContextualCards.addAll(allCards);
@@ -137,13 +138,15 @@ public class ContextualCardManager implements CardContentLoader.CardContentLoade
loadCardControllers(); loadCardControllers();
if (mListener != null) { if (mListener != null) {
mListener.onContextualCardUpdated(mContextualCards); final Map<Integer, List<ContextualCard>> cardsToUpdate = new ArrayMap<>();
cardsToUpdate.put(ContextualCard.CardType.DEFAULT, mContextualCards);
mListener.onContextualCardUpdated(cardsToUpdate);
} }
} }
@Override @Override
public void onFinishCardLoading(List<ContextualCard> contextualCards) { public void onFinishCardLoading(List<ContextualCard> cards) {
onContextualCardUpdated(contextualCards); onContextualCardUpdated(cards.stream().collect(groupingBy(ContextualCard::getCardType)));
} }
void setListener(ContextualCardUpdateListener listener) { void setListener(ContextualCardUpdateListener listener) {

View File

@@ -17,15 +17,19 @@
package com.android.settings.homepage; package com.android.settings.homepage;
import java.util.List; import java.util.List;
import java.util.Map;
/** /**
* When {@link ContextualCardController} detects changes, it will notify the listeners registered. * When {@link ContextualCardController} detects changes, it will notify the listeners registered.
* In our case, {@link ContextualCardManager} gets noticed.
*
* After the list of {@link ContextualCard} gets updated in{@link ContextualCardManager},
* {@link ContextualCardManager} will notify the listeners registered, {@link
* ContextualCardsAdapter} in this case.
*/ */
public interface ContextualCardUpdateListener { public interface ContextualCardUpdateListener {
void onContextualCardUpdated(List<ContextualCard> updateList);
/**
* Called when a set of cards are updated.
*
* @param cards A map of updates grouped by {@link ContextualCard.CardType}. Values can be
* null, which means all cards from corresponding {@link
* ContextualCard.CardType} are removed.
*/
void onContextualCardUpdated(Map<Integer, List<ContextualCard>> cards);
} }

View File

@@ -26,6 +26,7 @@ import androidx.recyclerview.widget.RecyclerView;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Map;
public class ContextualCardsAdapter extends RecyclerView.Adapter<RecyclerView.ViewHolder> public class ContextualCardsAdapter extends RecyclerView.Adapter<RecyclerView.ViewHolder>
implements ContextualCardUpdateListener { implements ContextualCardUpdateListener {
@@ -101,7 +102,8 @@ public class ContextualCardsAdapter extends RecyclerView.Adapter<RecyclerView.Vi
} }
@Override @Override
public void onContextualCardUpdated(List<ContextualCard> contextualCards) { public void onContextualCardUpdated(Map<Integer, List<ContextualCard>> cards) {
final List<ContextualCard> contextualCards = cards.get(ContextualCard.CardType.DEFAULT);
//TODO(b/112245748): Should implement a DiffCallback so we can use notifyItemChanged() //TODO(b/112245748): Should implement a DiffCallback so we can use notifyItemChanged()
// instead. // instead.
if (contextualCards == null) { if (contextualCards == null) {

View File

@@ -17,6 +17,7 @@
package com.android.settings.homepage.conditional; package com.android.settings.homepage.conditional;
import android.content.Context; import android.content.Context;
import android.util.ArrayMap;
import com.android.settings.homepage.ContextualCard; import com.android.settings.homepage.ContextualCard;
import com.android.settings.homepage.ContextualCardController; import com.android.settings.homepage.ContextualCardController;
@@ -27,6 +28,7 @@ import com.android.settingslib.core.lifecycle.events.OnStop;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.List; import java.util.List;
import java.util.Map;
/** /**
* This controller triggers the loading of conditional cards and monitors state changes to * This controller triggers the loading of conditional cards and monitors state changes to
@@ -56,11 +58,6 @@ public class ConditionContextualCardController implements ContextualCardControll
return ContextualCard.CardType.CONDITIONAL; return ContextualCard.CardType.CONDITIONAL;
} }
@Override
public void onDataUpdated(List<ContextualCard> cardList) {
mListener.onContextualCardUpdated(cardList);
}
@Override @Override
public void onStart() { public void onStart() {
mConditionManager.startMonitoringStateChange(); mConditionManager.startMonitoringStateChange();
@@ -105,7 +102,9 @@ public class ConditionContextualCardController implements ContextualCardControll
} }
if (mListener != null) { if (mListener != null) {
onDataUpdated(conditionCards); final Map<Integer, List<ContextualCard>> conditionalCards = new ArrayMap<>();
conditionalCards.put(ContextualCard.CardType.CONDITIONAL, conditionCards);
mListener.onContextualCardUpdated(conditionalCards);
} }
} }
} }

View File

@@ -82,7 +82,7 @@ public class ConditionContextualCardControllerTest {
mController.onConditionsChanged(); mController.onConditionsChanged();
verify(mController).onDataUpdated(any()); verify(mListener).onContextualCardUpdated(any());
} }
@Test @Test
@@ -94,7 +94,7 @@ public class ConditionContextualCardControllerTest {
mController.onConditionsChanged(); mController.onConditionsChanged();
verify(mController, never()).onDataUpdated(any()); verify(mListener, never()).onContextualCardUpdated(any());
} }
private class FakeConditionalCard implements ConditionalCard { private class FakeConditionalCard implements ConditionalCard {