Skip to content

feat: Add visibility reminder dialog - #2992

Open
Elouan1411 wants to merge 4 commits into
custom-reminderfrom
select-visibility
Open

feat: Add visibility reminder dialog#2992
Elouan1411 wants to merge 4 commits into
custom-reminderfrom
select-visibility

Conversation

@Elouan1411

Copy link
Copy Markdown
Contributor

No description provided.

@Elouan1411 Elouan1411 changed the title Select visibility feat: Add visibility reminder dialog and integrate with scheduling options Jul 22, 2026
@Elouan1411 Elouan1411 changed the title feat: Add visibility reminder dialog and integrate with scheduling options feat: Add visibility reminder dialog Jul 22, 2026
@Elouan1411
Elouan1411 requested a review from Copilot July 22, 2026 14:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a localized reminder-visibility dialog to the message send-options flow.

Changes:

  • Adds “Me only” translations across all supported locales.
  • Introduces the radio-selection dialog and layout.
  • Stores and displays the selected visibility in the send-options state.

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
app/src/main/res/values/strings.xml Adds English label.
app/src/main/res/values-sv/strings.xml Adds Swedish label.
app/src/main/res/values-pt/strings.xml Adds Portuguese label.
app/src/main/res/values-pl/strings.xml Adds Polish label.
app/src/main/res/values-nl/strings.xml Adds Dutch label.
app/src/main/res/values-nb/strings.xml Adds Norwegian label.
app/src/main/res/values-it/strings.xml Adds Italian label.
app/src/main/res/values-fr/strings.xml Adds French label.
app/src/main/res/values-fi/strings.xml Adds Finnish label.
app/src/main/res/values-es/strings.xml Adds Spanish label.
app/src/main/res/values-el/strings.xml Adds Greek label.
app/src/main/res/values-de/strings.xml Adds German label.
app/src/main/res/values-da/strings.xml Adds Danish label.
app/src/main/res/layout/dialog_select_visibility_reminder.xml Defines the visibility choices.
DraftSendOptionsFragment.kt Integrates selection and state handling.
NewMessageViewModel.kt Holds the selected visibility.
SelectVisibilityReminderDialog.kt Implements the selection dialog.

@Elouan1411
Elouan1411 force-pushed the select-visibility branch 2 times, most recently from 287df90 to 099759a Compare July 28, 2026 09:25
private fun initDialog() = with(binding) {
MaterialAlertDialogBuilder(activityContext)
.setTitle(R.string.reminderVisibilityTitle)
.setView(root)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're only using binding for root, you don't need your with.

~ You should have received a copy of the GNU General Public License
~ along with this program. If not, see <http://www.gnu.org/licenses/>.
-->
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you could use a LinearLayout. You could use the SettingRadioGroupView as the root view.


val scheduleConfig: MutableLiveData<ScheduleConfig> = MutableLiveData(ScheduleConfig.None)
val reminderConfig: MutableLiveData<ReminderConfig> = MutableLiveData(ReminderConfig.None)
val shouldRemindRecipient: MutableLiveData<Boolean> = MutableLiveData(true)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this a LiveData ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to a flow

setReminderOptionsVisible(isVisible = false)
setScheduleOptionsVisible(isVisible = false)

pendingShouldRemindRecipient = newMessageViewModel.shouldRemindRecipient.value ?: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reasons to use another value instead of newMessageViewModel.shouldRemindRecipient?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was used to work with the save button, but we removed that behavior

Comment on lines +75 to +86
private fun setupListeners() = with(binding) {
reminderVisibilityGroup.onItemCheckedListener { id, _, _ ->
this@SelectVisibilityReminderDialog.isRecipientsAndMeSelected = (id == R.id.selectionReminderRecipientsAndMe)
}

positiveButton.setOnClickListener {
onVisibilitySelected?.invoke(this@SelectVisibilityReminderDialog.isRecipientsAndMeSelected)
alertDialog.dismiss()
}

negativeButton.setOnClickListener { alertDialog.cancel() }
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private fun setupListeners() = with(binding) {
reminderVisibilityGroup.onItemCheckedListener { id, _, _ ->
this@SelectVisibilityReminderDialog.isRecipientsAndMeSelected = (id == R.id.selectionReminderRecipientsAndMe)
}
positiveButton.setOnClickListener {
onVisibilitySelected?.invoke(this@SelectVisibilityReminderDialog.isRecipientsAndMeSelected)
alertDialog.dismiss()
}
negativeButton.setOnClickListener { alertDialog.cancel() }
}
private fun setupListeners() {
binding.reminderVisibilityGroup.onItemCheckedListener { id, _, _ ->
isRecipientsAndMeSelected = id == R.id.selectionReminderRecipientsAndMe
}
positiveButton.setOnClickListener {
onVisibilitySelected?.invoke(isRecipientsAndMeSelected)
alertDialog.dismiss()
}
negativeButton.setOnClickListener { alertDialog.cancel() }
}

Comment on lines +87 to +91
private var shouldRemindRecipient: Boolean
get() = newMessageViewModel.shouldRemindRecipient.value ?: true
set(value) {
newMessageViewModel.shouldRemindRecipient.value = value
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As the other, it should be a stateFlow in the viewModel that's observed and update the UI accordingly

@Elouan1411
Elouan1411 force-pushed the select-visibility branch 2 times, most recently from 1942087 to c2842b3 Compare July 31, 2026 11:44
@sonarqubecloud

Copy link
Copy Markdown

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.

4 participants