Skip to content

feat: Add toggle for 3rd row message previews - #312

Open
brmiller wants to merge 1 commit into
maathimself:mainfrom
brmiller:disableMessagePreview
Open

feat: Add toggle for 3rd row message previews#312
brmiller wants to merge 1 commit into
maathimself:mainfrom
brmiller:disableMessagePreview

Conversation

@brmiller

@brmiller brmiller commented Jul 25, 2026

Copy link
Copy Markdown

Summary

Per Issue #311 , add settings ability to disable message previews

Changes

Adds toggle to Settings | Appearance | Layout | Message List to disable message previews.

Testing

Included index.test.js (not sure if you do test files this way) to test local storage; live compile, re-deploy, re-run, manual testing of toggle on and off, displaying in different layout styles.

Contributor License Agreement

By submitting this pull request I confirm that:

  • I have read and agree to the Contributor License Agreement.
  • My contribution is my own original work (or I have identified any
    third-party material and confirmed it is compatible with the CLA).
  • I have the right to submit this contribution under the terms of the CLA.

@brmiller

Copy link
Copy Markdown
Author

Yes, I got excited and did not follow all the guidelines in the CONTRIBUTING.md. If you approve the idea of the issue, I will fix up my commit to use conventional commit style. I can also rename my branch if necessary.

@maathimself

Copy link
Copy Markdown
Owner

Thanks for this, two things currently fail the frontend test job (I ran npm test on the branch):

  1. The store test fails. node --test on store/index.test.js throws ERR_IMPORT_ATTRIBUTE_MISSING for locales/en.json: importing the store transitively pulls in en.json, which Vite handles but raw node --test (what CI uses) rejects without a type: json attribute. That's why there's no existing store test. Please drop store/index.test.js, or, if you want coverage, extract the toggle logic into a small pure helper and test that instead of importing the whole store.

  2. The new strings need every locale. The five new admin.messageList.* keys are only in en.json, which fails the existing i18n locale files test (it enforces key parity across locales) and leaves non-English users on English labels. Please add them to the other six (fr, es, it, de, ru, zh-CN).

With those two, npm test goes green. Two more for completeness:

  1. Gate the expanded-thread previews too. There are three snippet renders in MessageList.jsx; the PR gates ThreadRow and MessageRow but not the expanded-thread sub-rows (the {msg.snippet || ''} near line 4135). With previews off, expanding a thread still shows a preview per sub-message. Please wrap that one in showMessagePreviews as well.

  2. Persist the preference server-side. schedulePrefSave({ showMessagePreviews }) is sent, but PATCH /auth/preferences (backend/src/routes/auth.js) whitelists a fixed set of keys and doesn't include showMessagePreviews, so it's dropped: the toggle only lives in localStorage per device and won't sync across devices. Please add it alongside the sibling prefs (hoverQuickActions, showMobileAvatars) in that handler.

Appreciate you picking this up. Once those are in I'll re-review and get it merged.

@brmiller
brmiller force-pushed the disableMessagePreview branch from 6816be2 to 303517e Compare July 27, 2026 04:11
@brmiller

Copy link
Copy Markdown
Author
  1. The store test fails. node --test on store/index.test.js... Please drop store/index.test.js...

Dropped.

  1. The new strings need every locale. ... Please add them to the other six (fr, es, it, de, ru, zh-CN).

I am not a speaker of these languages. Rather than using a plain "google translate", I used an AI with some context of what I was doing and asked it to provide translations. Hopefully this is accurate. If not, let me know how I might get better translations.

  1. Gate the expanded-thread previews too. There are three snippet renders in MessageList.jsx; the PR gates ThreadRow and MessageRow but not the expanded-thread sub-rows (the {msg.snippet || ''} near line 4135).

Good catch! I did not know this feature did that as I do not use it. :D Should be fixed.

  1. Persist the preference server-side. schedulePrefSave({ showMessagePreviews }) is sent, but ... whitelists a fixed set of keys and doesn't include showMessagePreviews.

Added.

@maathimself

Copy link
Copy Markdown
Owner

Thanks! One thing before I can merge: the showMessagePreviews pref is read in auth.js but never written to the DB (it's missing from the UPDATE query and its params), so it won't persist or sync across devices. Can you wire it into the preferences UPDATE like the other prefs? Or if it's meant to be local-only, just drop the auth.js change. Thanks!

@brmiller
brmiller force-pushed the disableMessagePreview branch from 303517e to 09b77a6 Compare August 1, 2026 04:56
@brmiller

brmiller commented Aug 1, 2026

Copy link
Copy Markdown
Author

@maathimself Take another look and thanks for your patience! I think I have the UPDATE statement updated now. I checked the database before and after multiple toggles, and it seems to be getting saved.

I also had a messy merge conflict because a) I had added showMessagePreview in the middle of some of the lists and b) some of the gravatar stuff has been removed from some of those lists. I ended up just re-adding the message preview methods names at the end of the lists which is possibly what I should have done from the beginning. So if you'll check I didn't actually remove something important, I'd appreciate it.

@brmiller brmiller changed the title (feat) Add toggle for 3rd row message previews feat: Add toggle for 3rd row message previews Aug 4, 2026
@brmiller
brmiller force-pushed the disableMessagePreview branch from 09b77a6 to 967bfe4 Compare August 5, 2026 05:11
@brmiller

brmiller commented Aug 5, 2026

Copy link
Copy Markdown
Author

Just a quick rebase - let me know if there's anything you'd like me to address!

@maathimself

Copy link
Copy Markdown
Owner

Thanks, the save side looks wired up right now.

One last thing and then I'll merge: it saves but never loads back. loadPreferences in store/index.js hydrates each pref from the server explicitly, one key at a time, and there's no block for showMessagePreviews. So on a second device (or after clearing localStorage) it falls back to the local default and the toggle doesn't actually sync. The GET returns the value fine, it's just never read into the store.

Mirror the showMobileAvatars block that's already in there:

if (typeof prefs.showMessagePreviews === 'boolean') {
localStorage.setItem('mailflow_show_message_previews', String(prefs.showMessagePreviews));
set({ showMessagePreviews: prefs.showMessagePreviews });
}

Drop that in next to the other avatar/favicon hydration blocks and it'll sync. Then it's good to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants