refactor: Implement trailing content management for schedule options and bottom sheets by Elouan1411 · Pull Request #2990 · Infomaniak/android-kMail

Elouan1411 · 2026-07-22T13:22:15Z

No description provided.

Copilot

Pull request overview

Refactors trailing-content handling across schedule options, settings rows, and bottom sheets.

Changes:

Extracts shared KSuite chip and trailing-content management.
Adds plan-specific chips to custom scheduling and reminder options.
Updates last-schedule-option visibility handling.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`DraftSendOptionsFragment.kt`	Integrates trailing content and custom reminder handling.
`MailActionsBottomSheetDialog.kt`	Updates the extracted trailing-content import.
`KSuiteChipManager.kt`	Adds reusable chip management and trailing-content types.
`ActionItemView.kt`	Delegates chip rendering to the shared manager.
`ItemSettingView.kt`	Supports chips and chevrons as trailing content.
`SimpleSchedulePickerBottomSheet.kt`	Configures plan-specific custom-option content.
`ScheduleOptionsHelper.kt`	Coordinates trailing content and option visibility.

Comments suppressed due to low confidence (1)

app/src/main/java/com/infomaniak/mail/ui/newMessage/sendOptions/DraftSendOptionsFragment.kt:271

onViewCreated() calls this with false immediately after ScheduleOptionsHelper computes whether the last option is eligible. That sets the child to GONE; a later call with true then evaluates true && false, so the last schedule option can never reappear. Hide only the wrapper and preserve the child's helper-computed visibility.

        lastScheduleOption.isVisible = isVisible && lastScheduleOption.isVisible

tevincent · 2026-07-28T11:59:14Z

        (firstItem as? ActionItemView)?.setDividerVisibility(shouldDisplayDivider)
    }

+    protected open fun setupCustomScheduleOptionTrailing(kSuite: KSuite?) {


Why did you put that as protected open?

Also you don't need the kSuite parameter as you can directly use currentKSuite

Why did you put that as protected open?

I forgot to remove it, it's no longer useful

tevincent · 2026-07-28T12:02:05Z

        chevron.isGone = mustBlock
    }

+    fun setMyKSuiteChipVisibility(isVisible: Boolean) {


This is never used ?

tevincent · 2026-07-28T12:03:32Z

+
+    private fun setTrailingContentUi(trailingContent: TrailingContent) = with(binding) {
+        val hasChip = kSuiteChipManager.displayChipFor(trailingContent)
+        trailingChipContainer.isVisible = hasChip


No need to create a hasCip variable here as you only use it here.

tevincent · 2026-07-28T12:11:50Z

+import com.infomaniak.core.ksuite.ksuitepro.views.EvolveChipView
+import com.infomaniak.core.ksuite.myksuite.ui.views.MyKSuitePlusChipView
+
+class KSuiteChipManager(private val container: ViewGroup) {


Don't pass a view like that as a val. It could leak. If you want the context, you could just pass it as a parameter:

class KSuiteChipManager(context: Context) { private val kSuitePersoChipView : BaseMyKSuiteChipView = MyKSuitePlusChipView(context) private val kSuiteProChipView : EvolveChipView = EvolveChipView(context) fun displayChipFor(container: ViewGroup, trailingContent: TrailingContent): Boolean { ...

and the by lazy is not necessary as you always use those variables.

Thanks 🙏

FabianDevel · 2026-07-28T12:58:58Z

        (firstItem as? ActionItemView)?.setDividerVisibility(shouldDisplayDivider)
    }

+    protected open fun setupCustomScheduleOptionTrailing(kSuite: KSuite?) {


Also you don't need the kSuite parameter as you can directly use currentKSuite

FabianDevel · 2026-07-28T13:00:45Z

+        binding.customScheduleOption.trailingContent = when (kSuite) {
+            KSuite.Perso.Free -> TrailingContent.KSuitePersoChip
+            KSuite.Pro.Free, KSuite.StarterPack -> TrailingContent.KSuiteProChip
+            else -> TrailingContent.Chevron
+        }
+    }


This could probably by factorized by a View extension function

FabianDevel · 2026-07-28T13:07:31Z

+    fun displayChipFor(trailingContent: TrailingContent): Boolean {
+        container.removeView(kSuitePersoChipView)
+        container.removeView(kSuiteProChipView)
+
+        return when (trailingContent) {
+            // ComposeView are not compatible with view without lifecycles (ex: PopupWindow in RecipientFieldView).
+            // This is causing a crash so to avoid that, we have to programmatically
+            // add the Compose view only where it's needed.
+            TrailingContent.KSuitePersoChip -> container.addView(kSuitePersoChipView).let { true }
+            TrailingContent.KSuiteProChip -> container.addView(kSuiteProChipView).let { true }
+            else -> false
+        }
+    }
+}


Here's my suggestion :

Suggested change

fun displayChipFor(trailingContent: TrailingContent): Boolean {

container.removeView(kSuitePersoChipView)

container.removeView(kSuiteProChipView)

return when (trailingContent) {

// ComposeView are not compatible with view without lifecycles (ex: PopupWindow in RecipientFieldView).

// This is causing a crash so to avoid that, we have to programmatically

// add the Compose view only where it's needed.

TrailingContent.KSuitePersoChip -> container.addView(kSuitePersoChipView).let { true }

TrailingContent.KSuiteProChip -> container.addView(kSuiteProChipView).let { true }

else -> false

}

}

}

fun displayChipFor(trailingContent: TrailingContent): Boolean {

container.removeView(kSuitePersoChipView)

container.removeView(kSuiteProChipView)

val chipView = when (trailingContent) {

// ComposeView are not compatible with view without lifecycles (ex: PopupWindow in RecipientFieldView).

// This is causing a crash so to avoid that, we have to programmatically

// add the Compose view only where it's needed.

TrailingContent.KSuitePersoChip -> kSuitePersoChipView

TrailingContent.KSuiteProChip -> kSuiteProChipView

else -> null

}

return chipView?.also(container::addView) != null

}

@tevincent @Elouan1411 ?

FabianDevel · 2026-07-28T13:09:18Z

+}
+
+/** Keep the entries order, it's used by the attribute (or change also the attributes order in attrs.xml) */
+enum class TrailingContent {


I'm don't think this should be kept in this class because it's not only related to kSuite chips

Or the class should be renamed and used more generally

FabianDevel · 2026-07-28T13:35:13Z

+    private fun onCustomDelayReminderClicked() =
+        executeIfAuthorized(MatomoName.ReminderCustomDelta.value) { showCustomDelayReminderDatePicker() }


Suggested change

private fun onCustomDelayReminderClicked() =

executeIfAuthorized(MatomoName.ReminderCustomDelta.value) { showCustomDelayReminderDatePicker() }

private fun onCustomDelayReminderClicked() {

executeIfAuthorized(MatomoName.ReminderCustomDelta.value) { showCustomDelayReminderDatePicker() }

}

FabianDevel · 2026-07-28T13:38:32Z

        ScheduleOptionUtils.getAvailableScheduleOptions(currentlyScheduledEpochMillis).forEach { scheduleOption ->
            scheduleOptions.addView(createScheduleOptionItem(scheduleOption))
        }
-


…and bottom sheets

sonarqubecloud · 2026-08-07T13:07:29Z

Quality Gate passed

Issues
0 New issues
1 Accepted issue

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions Bot assigned Elouan1411 Jul 22, 2026

Elouan1411 changed the title ~~refactor: Implement trailing content management for schedule options …~~ refactor: Implement trailing content management for schedule options and bottom sheets Jul 22, 2026

Elouan1411 requested a review from Copilot July 22, 2026 13:22

Copilot started reviewing on behalf of Elouan1411 July 22, 2026 13:23 View session

Elouan1411 force-pushed the trailing-content branch from 90ceb0e to b20f175 Compare July 22, 2026 13:24

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread app/src/main/java/com/infomaniak/mail/ui/newMessage/sendOptions/DraftSendOptionsFragment.kt

Comment thread app/src/main/java/com/infomaniak/mail/ui/newMessage/sendOptions/DraftSendOptionsFragment.kt Outdated

Elouan1411 force-pushed the trailing-content branch from b20f175 to 10a817b Compare July 23, 2026 08:55

Elouan1411 force-pushed the trailing-content branch 4 times, most recently from d7c3db2 to 6083e12 Compare July 28, 2026 09:25

tevincent requested changes Jul 28, 2026

View reviewed changes

FabianDevel requested changes Jul 28, 2026

View reviewed changes

Elouan1411 force-pushed the trailing-content branch from 6083e12 to d73f3f2 Compare July 28, 2026 14:05

Elouan1411 force-pushed the trailing-content branch 2 times, most recently from 478c028 to c5058ec Compare July 29, 2026 14:16

Elouan1411 force-pushed the trailing-content branch from c5058ec to b363fbe Compare July 30, 2026 08:42

Elouan1411 force-pushed the trailing-content branch from b363fbe to 0e2b679 Compare July 30, 2026 09:31

Elouan1411 force-pushed the trailing-content branch from 0e2b679 to 9e6918b Compare July 30, 2026 13:15

Elouan1411 force-pushed the trailing-content branch 2 times, most recently from 41a8670 to 784c3f3 Compare July 31, 2026 11:44

Elouan1411 force-pushed the trailing-content branch from 784c3f3 to 6957590 Compare July 31, 2026 12:33

Elouan1411 added 6 commits August 7, 2026 13:07

refactor: Implement trailing content management for schedule options …

c443306

…and bottom sheets

fix: Correct matomo

19e9b6d

refactor: Remove unnecessary code

895a49a

refactor: Clean code

c623b65

refactor: Create extension for trailing content

5a9fd02

fix: Prevent leak by using val for view

78269d4

Elouan1411 force-pushed the trailing-content branch from 6957590 to 78269d4 Compare August 7, 2026 13:02

		private fun onCustomDelayReminderClicked() =
		executeIfAuthorized(MatomoName.ReminderCustomDelta.value) { showCustomDelayReminderDatePicker() }

Uh oh!

Conversation

Elouan1411 commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Aug 7, 2026

Quality Gate passed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Elouan1411 commented Jul 22, 2026 •

edited

Loading