Merge pull request #177 from Pocket/notes-add-menu

fix(notes): show url/notes menu on add button click

Pocket/src/main/java/com/pocket/app/PocketUiPlaygroundActivity.java

@@ -110,18 +110,18 @@ private void bindExamples() {
 			}
 			new ThemedPopupMenu(v.getContext(),
 					ThemedPopupMenu.Section.radio("Theme", themeSelected, Arrays.asList(
-							new MenuItem(com.pocket.ui.R.string.light, 1, 0, view -> setThemeOverride(Theme.STATE_LIGHT, Theme.LIGHT)),
-							new MenuItem(com.pocket.ui.R.string.dark, 1, 0, view -> setThemeOverride(Theme.STATE_DARK, Theme.DARK)))),
+							new MenuItem(com.pocket.ui.R.string.light, 0, view -> setThemeOverride(Theme.STATE_LIGHT, Theme.LIGHT)),
+							new MenuItem(com.pocket.ui.R.string.dark, 0, view -> setThemeOverride(Theme.STATE_DARK, Theme.DARK)))),
 					ThemedPopupMenu.Section.radio("Interactions", isAllDisabled ? 1 : 0, Arrays.asList(
-							new MenuItem(R.string.dev_enabled, 1, 0, view -> setEnabledOverride(true)),
-							new MenuItem(R.string.dev_disabled, 1, 0, view -> setEnabledOverride(false)))),
+							new MenuItem(R.string.dev_enabled, 0, view -> setEnabledOverride(true)),
+							new MenuItem(R.string.dev_disabled, 0, view -> setEnabledOverride(false)))),
 					ThemedPopupMenu.Section.radio("Item Images", itemImagesState, Arrays.asList(
-							new MenuItem(R.string.dev_images, 1, 0, view -> setItemImagesState(0)),
-							new MenuItem(R.string.dev_no_images, 1, 0, view -> setItemImagesState(1)),
-							new MenuItem(R.string.dev_placeholders, 1, 0, view -> setItemImagesState(2)))),
+							new MenuItem(R.string.dev_images, 0, view -> setItemImagesState(0)),
+							new MenuItem(R.string.dev_no_images, 0, view -> setItemImagesState(1)),
+							new MenuItem(R.string.dev_placeholders, 0, view -> setItemImagesState(2)))),
 					ThemedPopupMenu.Section.radio("Item Mode", itemMode, Arrays.asList(
-							new MenuItem(R.string.dev_normal, 1, 0, view -> setItemMode(0)),
-							new MenuItem(R.string.dev_edit, 1, 0, view -> setItemMode(1))))
+							new MenuItem(R.string.dev_normal, 0, view -> setItemMode(0)),
+							new MenuItem(R.string.dev_edit, 0, view -> setItemMode(1))))
 			).show(v);
 		});
 

Pocket/src/main/java/com/pocket/app/list/MyListFragment.kt

@@ -41,6 +41,8 @@ import com.pocket.sdk.tts.Listen
 import com.pocket.sdk.util.AbsPocketFragment
 import com.pocket.sdk.util.dialog.AlertMessaging
 import com.pocket.sdk2.analytics.context.Interaction
+import com.pocket.ui.view.menu.MenuItem
+import com.pocket.ui.view.menu.ThemedPopupMenu
 import com.pocket.util.android.hideKeyboard
 import com.pocket.util.android.navigateSafely
 import com.pocket.util.android.repeatOnResumed
@@ -130,10 +132,30 @@ class MyListFragment : AbsPocketFragment() {
         return true
     }
 
-    @Suppress("LongMethod", "ComplexMethod")
     private fun setupNavigationEventListener() {
         viewModel.navigationEvents.collectWhenResumed(viewLifecycleOwner) { event ->
             when (event) {
+                is MyListNavigationEvent.ShowAddMenu -> {
+                    val context = context ?: return@collectWhenResumed
+                    val actions = ThemedPopupMenu.Section.actions(
+                        null,
+                        listOf(
+                            MenuItem(
+                                R.string.add_url_title,
+                                0,
+                                { viewModel.onAddUrlClicked() },
+                                null,
+                            ),
+                            MenuItem(
+                                R.string.add_note_title,
+                                0,
+                                { viewModel.onAddNoteClicked() },
+                                null,
+                            ),
+                        ),
+                    )
+                    ThemedPopupMenu(context, actions).show(binding.addButton)
+                }
                 is MyListNavigationEvent.ShowAddUrlBottomSheet -> {
                     AddUrlBottomSheetFragment.newInstance().show(
                         childFragmentManager,

Pocket/src/main/java/com/pocket/app/list/MyListViewModel.kt

@@ -8,6 +8,7 @@ import com.pocket.analytics.ContentOpenTracker
 import com.pocket.analytics.Tracker
 import com.pocket.analytics.appevents.SavesEvents
 import com.pocket.analytics.appevents.SavesTab
+import com.pocket.app.list.MyListViewModel.OnAddClick.*
 import com.pocket.app.list.list.ListManager
 import com.pocket.app.list.list.ListStatus
 import com.pocket.app.notes.Notes
@@ -80,20 +81,28 @@ class MyListViewModel @Inject constructor(
     // to get the current search text the list is using, try listManager.sortFilterState.value.search
     private var delayedSearchText = ""
 
+    private enum class OnAddClick { JustOpenAddUrl, ShowChoiceBetweenAddUrlAndAddNote }
+    private var onAddClick = JustOpenAddUrl
+
     init {
         setupListSortObserver()
         setupListManagerStateObserver()
         setupListObserver()
         setupTagListener()
         setupRecentSearchesListener()
         viewModelScope.launch {
+            val notesEnabled = notes.areEnabled()
             _uiState.update {
                 it.copy(
                     filterCarouselState = it.filterCarouselState.copy(
-                        notesFilterVisible = notes.areEnabled(),
+                        notesFilterVisible = notesEnabled,
                     ),
                 )
             }
+            onAddClick = when (notesEnabled) {
+                true -> ShowChoiceBetweenAddUrlAndAddNote
+                false -> JustOpenAddUrl
+            }
         }
     }
 
@@ -350,10 +359,27 @@ class MyListViewModel @Inject constructor(
     override fun onAddClicked() {
         tracker.track(SavesEvents.addButtonClicked())
         requireSignedIn {
-            _navigationEvents.tryEmit(MyListNavigationEvent.ShowAddUrlBottomSheet)
+            when (onAddClick) {
+                JustOpenAddUrl -> {
+                    _navigationEvents.tryEmit(MyListNavigationEvent.ShowAddUrlBottomSheet)
+                }
+                ShowChoiceBetweenAddUrlAndAddNote -> {
+                    _navigationEvents.tryEmit(MyListNavigationEvent.ShowAddMenu)
+                }
+            }
         }
     }
 
+    override fun onAddUrlClicked() {
+        // TODO(notes): tracker.track(…)
+        _navigationEvents.tryEmit(MyListNavigationEvent.ShowAddUrlBottomSheet)
+    }
+
+    override fun onAddNoteClicked() {
+        // TODO(notes): tracker.track(…)
+        // TODO(notes) POCKET-10881
+    }
+
     override fun onMyListChipClicked() {
         if (exitEditMode()) return
         tracker.track(SavesEvents.savesChipClicked())
@@ -434,7 +460,7 @@ class MyListViewModel @Inject constructor(
         }
     }
 
-    fun onNotesChipClicked() {
+    override fun onNotesChipClicked() {
         if (exitEditMode()) return
         // TODO(notes): tracker.track(…)
         requireSignedIn {
@@ -954,9 +980,10 @@ enum class BadgeType {
 }
 
 sealed class MyListNavigationEvent {
-    data object ShowAddUrlBottomSheet: MyListNavigationEvent()
-    object ShowTagBottomSheet: MyListNavigationEvent()
-    object ShowFilterBottomSheet: MyListNavigationEvent()
+    data object ShowAddMenu : MyListNavigationEvent()
+    data object ShowAddUrlBottomSheet : MyListNavigationEvent()
+    data object ShowTagBottomSheet : MyListNavigationEvent()
+    data object ShowFilterBottomSheet : MyListNavigationEvent()
 
     data class ShowBulkEditOverflowBottomSheet(
         val items: List<Item>
@@ -1001,12 +1028,15 @@ interface MyListInteractions {
     fun onFilterChipClicked()
     fun onBackButtonClicked(): Boolean
     fun onAddClicked()
+    fun onAddUrlClicked()
+    fun onAddNoteClicked()
     fun onMyListChipClicked()
     fun onArchiveChipClicked()
     fun onAllChipClicked()
     fun onTaggedChipClicked()
     fun onFavoritesChipClicked()
     fun onHighlightsChipClicked()
+    fun onNotesChipClicked()
     fun onEditChipClicked()
     fun onSelectedTagChipClicked()
     fun onSelectedFilterChipClicked()

Pocket/src/main/java/com/pocket/app/list/tags/TagBottomSheetFragment.kt

@@ -70,7 +70,6 @@ class TagBottomSheetFragment : AbsPocketBottomSheetDialogFragment() {
                     listOf(
                         MenuItem(
                             com.pocket.ui.R.string.ic_edit,
-                            R.menu.menu_tag_bottom_sheet_overflow,
                             com.pocket.ui.R.drawable.ic_pkt_pencil_line
                         ) {
                             viewModel.onEditClicked()

Pocket/src/main/java/com/pocket/app/reader/toolbar/OverflowBuilder.kt

@@ -7,7 +7,6 @@ import com.pocket.ui.view.menu.ThemedPopupMenu
 
 object OverflowBuilder {
 
-    @Suppress("LongMethod", "KotlinConstantConditions")
     fun showOverflow(
         anchorView: View,
         overflowUiState: ReaderToolbar.ToolbarOverflowUiState,
@@ -18,7 +17,6 @@ object OverflowBuilder {
         if (overflowUiState.textSettingsVisible) menuItems.add(
             MenuItem(
                 R.string.mu_display_settings,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_text_style_solid
             ) {
                 toolbarInteractions.onTextSettingsClicked()
@@ -28,7 +26,6 @@ object OverflowBuilder {
         if (overflowUiState.viewOriginalVisible) menuItems.add(
             MenuItem(
                 R.string.mu_view_original,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_web_view_line
             ) {
                 toolbarInteractions.onViewOriginalClicked()
@@ -38,7 +35,6 @@ object OverflowBuilder {
         if (overflowUiState.refreshVisible) menuItems.add(
             MenuItem(
                 R.string.mu_refresh,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_refresh_line
             ) {
                 toolbarInteractions.onRefreshClicked()
@@ -48,7 +44,6 @@ object OverflowBuilder {
         if (overflowUiState.findInPageVisible) menuItems.add(
             MenuItem(
                 R.string.mu_find_in_page,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_search_line
             ) {
                 toolbarInteractions.onFindInPageClicked()
@@ -58,7 +53,6 @@ object OverflowBuilder {
         if (overflowUiState.favoriteVisible) menuItems.add(
             MenuItem(
                 R.string.mu_favorite,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_favorite_line
             ) {
                 toolbarInteractions.onFavoriteClicked()
@@ -68,7 +62,6 @@ object OverflowBuilder {
         if (overflowUiState.unfavoriteVisible) menuItems.add(
             MenuItem(
                 R.string.mu_unfavorite,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_favorite_solid
             ) {
                 toolbarInteractions.onUnfavoriteClicked()
@@ -78,7 +71,6 @@ object OverflowBuilder {
         if (overflowUiState.addTagsVisible) menuItems.add(
             MenuItem(
                 R.string.mu_add_tags,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_add_tags_line
             ) {
                 toolbarInteractions.onAddTagsClicked()
@@ -88,7 +80,6 @@ object OverflowBuilder {
         if (overflowUiState.highlightsVisible) menuItems.add(
             MenuItem(
                 R.string.mu_annotations,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_highlights_line
             ) {
                 toolbarInteractions.onHighlightsClicked()
@@ -98,7 +89,6 @@ object OverflowBuilder {
         if (overflowUiState.markAsNotViewedVisible) menuItems.add(
             MenuItem(
                 com.pocket.ui.R.string.ic_mark_as_not_viewed,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_viewed_not
             ) {
                 toolbarInteractions.onMarkAsNotViewedClicked()
@@ -108,7 +98,6 @@ object OverflowBuilder {
         if (overflowUiState.deleteVisible) menuItems.add(
             MenuItem(
                 R.string.mu_delete,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_delete_line
             ) {
                 toolbarInteractions.onDeleteClicked()
@@ -118,7 +107,6 @@ object OverflowBuilder {
         if (overflowUiState.reportArticleVisible) menuItems.add(
             MenuItem(
                 R.string.mu_report_article_view,
-                menuItems.size,
                 com.pocket.ui.R.drawable.ic_pkt_error_line
             ) {
                 toolbarInteractions.onReportArticleClicked()

Pocket/src/main/res/values/strings.xml

@@ -626,7 +626,7 @@
 	<!-- All the remaining pieces of notes copy, for Smartling to translate in a single job. -->
 	<!-- TODO(notes): remove unused ignores -->
 	<string tools:ignore="UnusedResources" name="add_url_note_hint" comment="Hint for an input field where you can optionally add a note when add a URL to your saves">Note (optional)</string>
-	<string tools:ignore="UnusedResources" name="add_note_title">Add Note</string>
+	<string name="add_note_title">Add Note</string>
 	<string tools:ignore="UnusedResources" name="add_note_title_hint" comment="Hint for an input field to optionally add a title for a note you're creating">Title (optional)</string>
 	<string tools:ignore="UnusedResources" name="add_note_hint" comment="Hind for an input field where you put the body/content of a note you're creating">Note</string>
 	<string tools:ignore="UnusedResources" name="edit_note_title">Edit Note</string>

pocket-ui/src/main/java/com/pocket/ui/view/menu/MenuItem.java

@@ -8,20 +8,18 @@ public class MenuItem {
 
 	public final @StringRes int label;
 	public final int icon;
-	public final int id;
 	public final int groupId;
 	public final View.OnClickListener onClick;
 	public final String uiEntityIdentifier;
 
     private boolean mIsVisible = true;
     private boolean mIsEnabled = true;
 
-	public MenuItem(int label, int menuId, int icon, View.OnClickListener onClick) {
-		this(label, menuId, icon, onClick, null);
+	public MenuItem(int label, int icon, View.OnClickListener onClick) {
+		this(label, icon, onClick, null);
 	}
 
-	public MenuItem(int label, int menuId, int icon, View.OnClickListener onClick, String uiEntityIdentifier) {
-		this.id = menuId;
+	public MenuItem(int label, int icon, View.OnClickListener onClick, String uiEntityIdentifier) {
 		this.groupId = 1;
 		this.label = label;
 		this.icon = icon;
@@ -44,23 +42,20 @@ public void onClick(View v) {
 	}
 
 	/**
-	 * Invoked each time, right before the menu is displayed. Returns whether or
-	 * not this option should be visible in the menu.
-	 * 
-	 * @return
+	 * Invoked each time, right before the menu is displayed.
+	 *
+	 * @return whether or not this option should be visible in the menu
 	 */
 	public boolean isVisible() {
 		return mIsVisible;
 	}
 
 	/**
-	 * Invoked each time, right before the menu is displayed. Returns whether or
-	 * not this option should be enabled in the menu.
+	 * Invoked each time, right before the menu is displayed.
 	 * 
-	 * @return
+	 * @return whether or not this option should be enabled in the menu
 	 */
 	public boolean isEnabled() {
 		return mIsEnabled;
 	}
-
-}
+}