settings: Add option to mark read on scroll only in conversation views

Using the handy new InputRowRadioButtons component we added in
edf5322.

Fixes: #5241

Author: chrisbobbe

src/common/InputRowRadioButtons.js

@@ -58,7 +58,7 @@ type Props<TItemKey> = $ReadOnly<{|
 |}>;
 
 /**
- * A form-input row for the user to make a choice, radio-button style.
+ * An input row for the user to make a choice, radio-button style.
  *
  * Shows the current value (the selected item), represented as the item's
  * `.title`. When tapped, opens the selectable-options screen, where the

src/common/SwitchRow.js

@@ -18,8 +18,8 @@ type Props = $ReadOnly<{|
 const componentStyles = createStyleSheet({
   container: {
     // For uniformity with other rows this might share a screen with, e.g.,
-    // NestedNavRow on the settings screen. See height-related attributes on
-    // those rows.
+    // NestedNavRow and InputRowRadioButtons on the settings screen. See
+    // height-related attributes on those rows.
     minHeight: 48,
   },
   icon: {

src/reduxTypes.js

@@ -397,7 +397,7 @@ export type GlobalSettingsState = $ReadOnly<{
 
   // Possibly this should be per-account.  If so it should probably be put
   // on the server, so it can also be cross-device for the account.
-  doNotMarkMessagesAsRead: boolean,
+  markMessagesReadOnScroll: 'always' | 'never' | 'conversation-views-only',
 
   ...
 }>;

src/settings/SettingsScreen.js

@@ -8,6 +8,7 @@ import type { AppNavigationProp } from '../nav/AppNavigator';
 import { useGlobalSelector, useDispatch } from '../react-redux';
 import { getGlobalSettings } from '../selectors';
 import NestedNavRow from '../common/NestedNavRow';
+import InputRowRadioButtons from '../common/InputRowRadioButtons';
 import SwitchRow from '../common/SwitchRow';
 import Screen from '../common/Screen';
 import {
@@ -27,8 +28,8 @@ type Props = $ReadOnly<{|
 export default function SettingsScreen(props: Props): Node {
   const theme = useGlobalSelector(state => getGlobalSettings(state).theme);
   const browser = useGlobalSelector(state => getGlobalSettings(state).browser);
-  const doNotMarkMessagesAsRead = useGlobalSelector(
-    state => getGlobalSettings(state).doNotMarkMessagesAsRead,
+  const markMessagesReadOnScroll = useGlobalSelector(
+    state => getGlobalSettings(state).markMessagesReadOnScroll,
   );
   const dispatch = useDispatch();
   const { navigation } = props;
@@ -47,11 +48,23 @@ export default function SettingsScreen(props: Props): Node {
           dispatch(setGlobalSettings({ browser: value ? 'embedded' : 'external' }));
         }}
       />
-      <SwitchRow
-        label="Do not mark messages read on scroll"
-        value={doNotMarkMessagesAsRead}
+      <InputRowRadioButtons
+        navigation={navigation}
+        label="Mark messages as read on scroll"
+        description="When scrolling through messages, should they automatically be marked as read?"
+        valueKey={markMessagesReadOnScroll}
+        items={[
+          { key: 'always', title: 'Always' },
+          { key: 'never', title: 'Never' },
+          {
+            key: 'conversation-views-only',
+            title: 'Only in conversation views',
+            subtitle:
+              'Messages will be marked as read in single-topic or private-message views, but not in interleaved views, such as whole-stream views.',
+          },
+        ]}
         onValueChange={value => {
-          dispatch(setGlobalSettings({ doNotMarkMessagesAsRead: value }));
+          dispatch(setGlobalSettings({ markMessagesReadOnScroll: value }));
         }}
       />
       <NestedNavRow

src/settings/settingsReducer.js

@@ -18,7 +18,7 @@ const initialState: SettingsState = {
   theme: 'default',
   browser: 'default',
   experimentalFeaturesEnabled: false,
-  doNotMarkMessagesAsRead: false,
+  markMessagesReadOnScroll: 'always',
 
   //
   // PerAccountSettingsState

src/storage/__tests__/migrations-test.js

@@ -84,10 +84,27 @@ describe('migrations', () => {
     },
   };
 
+  // What `base` becomes after migrations up through 52.
+  const base52 = {
+    ...base37,
+    migrations: { version: 52 },
+    settings: {
+      language: 'en',
+      theme: 'default',
+      offlineNotification: true,
+      onlineNotification: true,
+      experimentalFeaturesEnabled: false,
+      streamNotification: false,
+      browser: 'default',
+      // renamed/retyped from boolean doNotMarkMessagesAsRead
+      markMessagesReadOnScroll: 'always',
+    },
+  };
+
   // What `base` becomes after all migrations.
   const endBase = {
-    ...base37,
-    migrations: { version: 51 },
+    ...base52,
+    migrations: { version: 52 },
   };
 
   for (const [desc, before, after] of [
@@ -206,12 +223,12 @@ describe('migrations', () => {
     [
       'check 37 with setting already false',
       { ...base15, settings: { ...base15.settings, doNotMarkMessagesAsRead: false } },
-      { ...endBase, settings: { ...endBase.settings, doNotMarkMessagesAsRead: false } },
+      { ...endBase, settings: { ...endBase.settings, markMessagesReadOnScroll: 'always' } },
     ],
     [
       'check 37 with setting already true',
       { ...base15, settings: { ...base15.settings, doNotMarkMessagesAsRead: true } },
-      { ...endBase, settings: { ...endBase.settings, doNotMarkMessagesAsRead: true } },
+      { ...endBase, settings: { ...endBase.settings, markMessagesReadOnScroll: 'never' } },
     ],
     [
       'check 38',
@@ -241,6 +258,24 @@ describe('migrations', () => {
         drafts: { 'topic:8:stuff': 'text', 'stream:8': 'more text', 'pm:12': 'pm text' },
       },
     ],
+    [
+      'check 52 with old setting false',
+      {
+        ...base37,
+        migrations: { version: 51 },
+        settings: { ...base37.settings, doNotMarkMessagesAsRead: false },
+      },
+      { ...endBase, settings: { ...endBase.settings, markMessagesReadOnScroll: 'always' } },
+    ],
+    [
+      'check 52 with old setting true',
+      {
+        ...base37,
+        migrations: { version: 51 },
+        settings: { ...base37.settings, doNotMarkMessagesAsRead: true },
+      },
+      { ...endBase, settings: { ...endBase.settings, markMessagesReadOnScroll: 'never' } },
+    ],
   ]) {
     /* eslint-disable no-loop-func */
     test(desc, async () => {

src/storage/migrations.js

@@ -371,12 +371,16 @@ const migrationsInner: {| [string]: (LessPartialState) => LessPartialState |} =
   // Handle explicitly the migrations above (before 30, 34, 36, and this
   // one) that were done implicitly by the behavior of `autoRehydrate` on a
   // REHYDRATE action.
+  /* $FlowIgnore[prop-missing]: `doNotMarkMessagesAsRead` renamed to
+     `markMessagesReadOnScroll` in 52 */
   '37': state => ({
     // This handles the migrations affecting RealmState, before 34, 36, and here.
     ...dropCache(state),
     // Handle the migration before 30.
     settings: {
       ...state.settings,
+      /* $FlowIgnore[prop-missing]: `doNotMarkMessagesAsRead` renamed to
+         `markMessagesReadOnScroll` in 52 */
       doNotMarkMessagesAsRead: state.settings.doNotMarkMessagesAsRead ?? false,
     },
   }),
@@ -443,6 +447,19 @@ const migrationsInner: {| [string]: (LessPartialState) => LessPartialState |} =
   // Add serverEmojiData to state.realm.
   '51': dropCache,
 
+  // Change boolean doNotMarkMessagesAsRead to enum markMessagesReadOnScroll
+  '52': state => {
+    // $FlowIgnore[prop-missing]
+    const { doNotMarkMessagesAsRead, ...restSettings } = state.settings;
+    return {
+      ...state,
+      settings: {
+        ...restSettings,
+        markMessagesReadOnScroll: (doNotMarkMessagesAsRead: boolean) ? 'never' : 'always',
+      },
+    };
+  },
+
   // TIP: When adding a migration, consider just using `dropCache`.
   //   (See its jsdoc for guidance on when that's the right answer.)
 };

src/webview/MessageList.js

@@ -35,8 +35,9 @@ import { handleWebViewOutboundEvent } from './handleOutboundEvents';
 import { base64Utf8Encode } from '../utils/encoding';
 import * as logging from '../utils/logging';
 import { tryParseUrl } from '../utils/url';
-import { caseNarrow } from '../utils/narrow';
+import { caseNarrow, isConversationNarrow } from '../utils/narrow';
 import { type BackgroundData, getBackgroundData } from './backgroundData';
+import { ensureUnreachable } from '../generics';
 
 type OuterProps = $ReadOnly<{|
   narrow: Narrow,
@@ -300,7 +301,20 @@ const MessageList: ComponentType<OuterProps> = connect<SelectorProps, _, _>(
       ),
       typingUsers: getCurrentTypingUsers(state, props.narrow),
       doNotMarkMessagesAsRead:
-        !marksMessagesAsRead(props.narrow) || globalSettings.doNotMarkMessagesAsRead,
+        !marksMessagesAsRead(props.narrow)
+        || (() => {
+          switch (globalSettings.markMessagesReadOnScroll) {
+            case 'always':
+              return false;
+            case 'never':
+              return true;
+            case 'conversation-views-only':
+              return !isConversationNarrow(props.narrow);
+            default:
+              ensureUnreachable(globalSettings.markMessagesReadOnScroll);
+              return false;
+          }
+        })(),
     };
   },
 )(connectActionSheet(withGetText(MessageListInner)));

src/webview/__tests__/generateInboundEvents-test.js

@@ -15,7 +15,7 @@ describe('generateInboundEvents', () => {
     messages: [],
     messageListElementsForShownMessages: [],
     typingUsers: [],
-    doNotMarkMessagesAsRead: eg.baseReduxState.settings.doNotMarkMessagesAsRead,
+    doNotMarkMessagesAsRead: false,
   });
 
   type FudgedProps = {|

static/translations/messages_en.json

@@ -283,7 +283,12 @@
   "Active": "Active",
   "Idle": "Idle",
   "Offline": "Offline",
-  "Do not mark messages read on scroll": "Do not mark messages read on scroll",
+  "Mark messages as read on scroll": "Mark messages as read on scroll",
+  "When scrolling through messages, should they automatically be marked as read?": "When scrolling through messages, should they automatically be marked as read?",
+  "Always": "Always",
+  "Never": "Never",
+  "Only in conversation views": "Only in conversation views",
+  "Messages will be marked as read in single-topic or private-message views, but not in interleaved views, such as whole-stream views.": "Messages will be marked as read in single-topic or private-message views, but not in interleaved views, such as whole-stream views.",
   "Topics": "Topics",
   "Add subscribers": "Add subscribers",
   "Subscribe": "Subscribe",