Skip to content

Clamped Disppearing Messages to 4 Weeks #5965

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

MarlowBrown
Copy link
Contributor

Contributor checklist

  • My commits are rebased on the latest main branch
  • My commits are in nice logical chunks
  • My contribution is fully baked and is ready to be merged as is
  • I have tested my contribution on these devices:
  • iPhone 16, iOS 18.2

Description

This pull request fixes #5947 by clamping an internal value in the Disappearing Messages Timers picker to 4 weeks.

…rval is weeks.

(cherry picked from commit 8908f61a989f0b3a1c454353c13fe97ac1222913)
(cherry picked from commit 87cdde7309f1aed437e14ac4cbef391c15c63404)
@sashaweiss-signal
Copy link
Contributor

Hi @MarlowBrown,

While I'm guessing this workaround works for the specific case of handling too-many weeks, it feels like we shouldn't need to be clamping. Instead, it feels like when the unit is changed (e.g., from days to weeks), and the maximum selectable number of units is clamped in the UI (e.g., from some number of days to "4", for weeks), that the new "selected" unit should be propagated back to the view controller by the CustomTimePicker.

(cherry picked from commit a58de504321a8dcea510db1b71596eb16d38aa49)
@MarlowBrown
Copy link
Contributor Author

Update for you @sashaweiss-signal

I changed the code to avoid clamping. The change is a little weird as technically until the user has moved the picker wheel, it doesn't count as "selected". I think it might be by design, so the code for force selecting is warranted.

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