Skip to content

Comments

[ADDED] Sonnet 3.7 generated improved log screen#196

Merged
hossain-khan merged 12 commits intomainfrom
improve-log-screen-by-ai
Mar 7, 2025
Merged

[ADDED] Sonnet 3.7 generated improved log screen#196
hossain-khan merged 12 commits intomainfrom
improve-log-screen-by-ai

Conversation

@hossain-khan
Copy link
Owner

@hossain-khan hossain-khan commented Mar 1, 2025

Take a look into AlertCheckLogViewerScreen.kt and improve the UI/UX of the screen. Add filtering and other applicable functionality to make it better for users.


This pull request includes several changes to the dev.hossain.remotenotify project, focusing on improving the user interface and refactoring utility functions. The most important changes include the addition of a new filter UI for alert logs, updating notification sender documentation, and refactoring time formatting utilities.

UI Enhancements:

  • Added a comprehensive filter UI for alert logs in FilterLogsUi.kt, allowing users to filter logs by alert type, notification method, and date range.

Documentation Updates:

  • Updated the NotificationSender interface documentation to include more details about the different types of remote destinations.

Refactoring:

  • Refactored time formatting functions by moving formatTimeElapsed and formatDuration to a new file DateTimeFormatter.kt and removing the old formatTimeDuration function [1] [2].
  • Updated references to the new formatTimeElapsed function in LastCheckStatusCardUi.kt [1] [2] [3].

Minor Changes:

  • Increased the default log limit in AlertCheckLogDao.kt from 20 to 30.
  • Added a new vector drawable resource filter_alt_24dp.xml.

@hossain-khan hossain-khan self-assigned this Mar 1, 2025
@hossain-khan
Copy link
Owner Author

Screen_recording_20250301_150401.mp4

@hossain-khan
Copy link
Owner Author

The log screen looks good. However, the search feature does not work well for this. Please remove the search feature from AlertCheckLogViewerScreen.kt

@hossain-khan
Copy link
Owner Author

toFJJYI9.jsonl.zip

@hossain-khan hossain-khan marked this pull request as draft March 2, 2025 02:34
@github-actions
Copy link

github-actions bot commented Mar 7, 2025

Code Coverage Report

View Coverage Report

@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2025

Codecov Report

Attention: Patch coverage is 25.77320% with 72 lines in your changes missing coverage. Please review.

Project coverage is 5.97%. Comparing base (b042a48) to head (492858b).
Report is 14 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff            @@
##            main    #196      +/-   ##
========================================
+ Coverage   3.23%   5.97%   +2.73%     
========================================
  Files         72      74       +2     
  Lines       1636    1690      +54     
  Branches      99     114      +15     
========================================
+ Hits          53     101      +48     
- Misses      1577    1583       +6     
  Partials       6       6              
🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@hossain-khan
Copy link
Owner Author

The fliter chip items are too many and does not fit in one row. Is there any way to break the filter chips to multiple lines if they don't fit? Follow the best practices.

@hossain-khan
Copy link
Owner Author

You're right that filter chips can overflow when there are too many items in a single row. I'll modify the code to allow filter chips to wrap to multiple lines.

Here's an improved implementation using FlowRow from Compose Material:

import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.ExperimentalLayoutApi
import androidx.compose.foundation.layout.FlowRow
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.padding

@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun NotifierTypeFilterChips(
    currentState: AlertCheckLogViewerScreen.State,
    onFilterByNotifierType: (NotifierType?) -> Unit,
    modifier: Modifier = Modifier,
) {
    FlowRow(
        horizontalArrangement = Arrangement.spacedBy(8.dp),
        verticalArrangement = Arrangement.spacedBy(8.dp),
        modifier = modifier
            .fillMaxWidth()
            .padding(bottom = 16.dp),
    ) {
        FilterChip(
            selected = currentState.selectedNotifierType == null,
            onClick = { onFilterByNotifierType(null) },
            label = { Text("All") },
        )

        // Create a chip for each notifier type
        NotifierType.entries.forEach { notifierType ->
            FilterChip(
                selected = currentState.selectedNotifierType == notifierType,
                onClick = { onFilterByNotifierType(notifierType) },
                label = { Text(notifierType.displayName) },
            )
        }
    }
}

Then replace your original Row with a call to this composable:

NotifierTypeFilterChips(
    currentState = currentState,
    onFilterByNotifierType = onFilterByNotifierType,
)

This implementation:

  1. Uses FlowRow instead of Row to automatically wrap content to multiple lines when needed
  2. Adds verticalArrangement to maintain proper spacing between rows
  3. Extracts the filter chips into a separate composable function for better reusability
  4. Preserves the functionality of your original implementation
  5. Adds proper spacing between chips in both horizontal and vertical directions

FlowRow is specifically designed for this use case where you want items to wrap naturally when they don't fit in a single row.

@hossain-khan
Copy link
Owner Author

On the ActiveFiltersSection UI, when the chip is removed it does not reve with animation. Also, when the whole section is reved from the list view (LazyColumn) it does not remove with animation.

Please add animation following the material 3 design best practices.

@hossain-khan
Copy link
Owner Author

Looking at your ActiveFiltersSection code, I'll add animations for both individual FilterChip removal and the entire section visibility. This will follow Material 3 design principles for smooth transitions.

Here's the updated code with animations:

@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun ActiveFiltersSection(
    showTriggeredOnly: Boolean,
    alertType: AlertType?,
    notifierType: NotifierType?,
    dateRange: Pair<Long?, Long?>,
    onClearFilter: (String) -> Unit,
) {
    Column(
        modifier = Modifier
            .fillMaxWidth()
            .padding(bottom = 8.dp)
            .animateContentSize(
                animationSpec = spring(
                    dampingRatio = Spring.DampingRatioMediumBouncy,
                    stiffness = Spring.StiffnessMedium
                )
            ),
    ) {
        Text(
            text = "Active Filters:",
            style = MaterialTheme.typography.labelLarge,
            color = MaterialTheme.colorScheme.primary,
            modifier = Modifier.padding(bottom = 4.dp),
        )

        FlowRow(
            horizontalArrangement = Arrangement.spacedBy(8.dp),
            verticalArrangement = Arrangement.spacedBy(8.dp),
            modifier = Modifier.padding(vertical = 4.dp),
        ) {
            // Triggered Only Filter
            AnimatedVisibility(
                visible = showTriggeredOnly,
                enter = fadeIn() + expandHorizontally(),
                exit = fadeOut() + shrinkHorizontally()
            ) {
                FilterChip(
                    selected = true,
                    onClick = { onClearFilter("triggered") },
                    label = { Text("Triggered Only") },
                    trailingIcon = {
                        Icon(
                            imageVector = Icons.Default.Clear,
                            contentDescription = "Clear Filter",
                            modifier = Modifier.size(16.dp),
                        )
                    },
                )
            }

            // Alert Type Filter
            AnimatedVisibility(
                visible = alertType != null,
                enter = fadeIn() + expandHorizontally(),
                exit = fadeOut() + shrinkHorizontally()
            ) {
                FilterChip(
                    selected = true,
                    onClick = { onClearFilter("alertType") },
                    label = {
                        Text(
                            when (alertType) {
                                AlertType.BATTERY -> "Battery"
                                AlertType.STORAGE -> "Storage"
                                else -> ""
                            },
                        )
                    },
                    trailingIcon = {
                        Icon(
                            imageVector = Icons.Default.Clear,
                            contentDescription = "Clear Filter",
                            modifier = Modifier.size(16.dp),
                        )
                    },
                )
            }

            // Notifier Type Filter
            AnimatedVisibility(
                visible = notifierType != null,
                enter = fadeIn() + expandHorizontally(),
                exit = fadeOut() + shrinkHorizontally()
            ) {
                FilterChip(
                    selected = true,
                    onClick = { onClearFilter("notifierType") },
                    label = { Text(notifierType?.displayName ?: "") },
                    trailingIcon = {
                        Icon(
                            imageVector = Icons.Default.Clear,
                            contentDescription = "Clear Filter",
                            modifier = Modifier.size(16.dp),
                        )
                    },
                )
            }

            // Date Range Filter
            AnimatedVisibility(
                visible = dateRange.first != null || dateRange.second != null,
                enter = fadeIn() + expandHorizontally(),
                exit = fadeOut() + shrinkHorizontally()
            ) {
                FilterChip(
                    selected = true,
                    onClick = { onClearFilter("dateRange") },
                    label = {
                        val dateText =
                            when {
                                dateRange.first != null && dateRange.second != null -> "Date Range"
                                dateRange.first != null ->
                                    "From ${
                                        SimpleDateFormat("MMM dd", Locale.getDefault())
                                            .format(Date(dateRange.first!!))
                                    }"
                                else ->
                                    "Until ${
                                        SimpleDateFormat("MMM dd", Locale.getDefault())
                                            .format(Date(dateRange.second!!))
                                    }"
                            }
                        Text(dateText)
                    },
                    trailingIcon = {
                        Icon(
                            imageVector = Icons.Default.Clear,
                            contentDescription = "Clear Filter",
                            modifier = Modifier.size(16.dp),
                        )
                    },
                )
            }
        }
    }
}

Changes made:

  1. Added animateContentSize() to the parent Column to animate the whole section when filters change
  2. Replaced Row with FlowRow to better handle filter chips wrapping to multiple lines
  3. Wrapped each FilterChip with AnimatedVisibility for individual chip animations
  4. Used combined animations (fadeIn() + expandHorizontally() for enter, fadeOut() + shrinkHorizontally() for exit) for smooth transitions
  5. Used Spring animation spec for a more natural feel following Material 3 motion guidelines

Make sure to import these additional dependencies:

import androidx.compose.animation.AnimatedVisibility
import androidx.compose.animation.animateContentSize
import androidx.compose.animation.expandHorizontally
import androidx.compose.animation.fadeIn
import androidx.compose.animation.fadeOut
import androidx.compose.animation.shrinkHorizontally
import androidx.compose.foundation.layout.ExperimentalLayoutApi
import androidx.compose.foundation.layout.FlowRow
import androidx.compose.animation.core.Spring
import androidx.compose.animation.core.spring

@hossain-khan
Copy link
Owner Author

When the item(key = "active_filters") is removed from the LazyColumn it doesn't animate. How do I enable animation for item add and remove for LazyColumn?

@hossain-khan
Copy link
Owner Author

To enable animation for adding and removing items in a LazyColumn, you'll need to use Jetpack Compose's animateItemPlacement modifier. This will animate the position changes when items are added or removed from the list.

Here's how to implement it:

import androidx.compose.foundation.lazy.items
import androidx.compose.animation.core.tween
import androidx.compose.foundation.ExperimentalFoundationApi

@OptIn(ExperimentalFoundationApi::class)
LazyColumn(
    state = listState,
    modifier = Modifier.fillMaxSize(),
    verticalArrangement = Arrangement.spacedBy(8.dp),
    contentPadding = PaddingValues(16.dp),
) {
    item(key = "logs_summary") {
        LogsSummaryInfo(
            totalLogs = state.logs.size,
            filteredLogs = state.filteredLogs.size,
            checkIntervalMinutes = state.checkIntervalMinutes,
            hasActiveFilters = hasActiveFilters,
            onClearFilters = {
                if (hasActiveFilters) {
                    state.eventSink(AlertCheckLogViewerScreen.Event.ClearFilters)
                }
            },
            modifier = Modifier.animateItemPlacement(
                animationSpec = tween(durationMillis = 300)
            )
        )
    }

    // Active filters display
    if (hasActiveFilters) {
        item(key = "active_filters") {
            ActiveFiltersSection(
                showTriggeredOnly = state.showTriggeredOnly,
                alertType = state.selectedAlertType,
                notifierType = state.selectedNotifierType,
                dateRange = state.dateRange,
                onClearFilter = { filter ->
                    when (filter) {
                        "triggered" -> state.eventSink(AlertCheckLogViewerScreen.Event.ToggleTriggeredOnly)
                        "alertType" -> state.eventSink(AlertCheckLogViewerScreen.Event.FilterByAlertType(null))
                        "notifierType" -> state.eventSink(AlertCheckLogViewerScreen.Event.FilterByNotifierType(null))
                        "dateRange" -> state.eventSink(AlertCheckLogViewerScreen.Event.FilterByDateRange(null, null))
                    }
                },
                modifier = Modifier.animateItemPlacement(
                    animationSpec = tween(durationMillis = 300)
                )
            )
        }
    }

    items(
        count = state.filteredLogs.size,
        key = { index -> state.filteredLogs[index].checkedOn },
    ) { index ->
        val log = state.filteredLogs[index]
        val isExpanded = expandedLogId == log.checkedOn

        LogItemCard(
            log = log,
            isExpanded = isExpanded,
            onClick = {
                expandedLogId = if (isExpanded) -1L else log.checkedOn
            },
            onCopyDetails = {
                // Copy functionality
            },
            modifier = Modifier.animateItemPlacement(
                animationSpec = tween(durationMillis = 300)
            )
        )
    }
}

Key changes made:

  1. Added @OptIn(ExperimentalFoundationApi::class) since animateItemPlacement is experimental
  2. Added modifier = Modifier.animateItemPlacement() to each item in the LazyColumn
  3. Used tween animation spec to specify the duration and easing of the animation
  4. Ensured all items have the animation, including the summary, active filters section, and each log item
  5. Added a modifier parameter to each composable function to pass the animation modifier

Don't forget to update your LogItemCard and other composable functions to accept and use the modifier parameter:

@Composable
private fun LogItemCard(
    log: AlertCheckLog,
    isExpanded: Boolean,
    onClick: () -> Unit,
    onCopyDetails: () -> Unit,
    modifier: Modifier = Modifier
) {
    // Use the provided modifier combined with your existing modifiers
    Card(
        modifier = modifier
            .fillMaxWidth()
            .clickable(onClick = onClick),
        // rest of implementation
    ) {
        // ...
    }
}

Make similar adjustments to your LogsSummaryInfo and ActiveFiltersSection composables.

@hossain-khan
Copy link
Owner Author

is there a LazyColumn modifier that would do the animation? I think the animateItemPlacement is deprecated.

@hossain-khan
Copy link
Owner Author

How about using Modifier.animateItem() on the item itself?

@hossain-khan
Copy link
Owner Author

To animate items being added or removed from a LazyColumn, you should use the animateItemPlacement() modifier. While you mentioned Modifier.animateItem(), the correct API is Modifier.animateItemPlacement().

Here's how to implement it for your LazyColumn:

import androidx.compose.foundation.ExperimentalFoundationApi
import androidx.compose.animation.core.tween

@OptIn(ExperimentalFoundationApi::class)
LazyColumn(
    state = listState,
    modifier = Modifier.fillMaxSize(),
    verticalArrangement = Arrangement.spacedBy(8.dp),
    contentPadding = PaddingValues(16.dp),
) {
    item(key = "logs_summary") {
        LogsSummaryInfo(
            totalLogs = state.logs.size,
            filteredLogs = state.filteredLogs.size,
            checkIntervalMinutes = state.checkIntervalMinutes,
            hasActiveFilters = hasActiveFilters,
            onClearFilters = {
                if (hasActiveFilters) {
                    state.eventSink(AlertCheckLogViewerScreen.Event.ClearFilters)
                }
            },
            modifier = Modifier.animateItemPlacement(
                animationSpec = tween(durationMillis = 300)
            )
        )
    }

    // Active filters display
    if (hasActiveFilters) {
        item(key = "active_filters") {
            ActiveFiltersSection(
                showTriggeredOnly = state.showTriggeredOnly,
                alertType = state.selectedAlertType,
                notifierType = state.selectedNotifierType,
                dateRange = state.dateRange,
                onClearFilter = { filter ->
                    when (filter) {
                        "triggered" -> state.eventSink(AlertCheckLogViewerScreen.Event.ToggleTriggeredOnly)
                        "alertType" -> state.eventSink(AlertCheckLogViewerScreen.Event.FilterByAlertType(null))
                        "notifierType" -> state.eventSink(AlertCheckLogViewerScreen.Event.FilterByNotifierType(null))
                        "dateRange" -> state.eventSink(AlertCheckLogViewerScreen.Event.FilterByDateRange(null, null))
                    }
                },
                modifier = Modifier.animateItemPlacement(
                    animationSpec = tween(durationMillis = 300)
                )
            )
        }
    }

    items(
        count = state.filteredLogs.size,
        key = { index -> state.filteredLogs[index].checkedOn },
    ) { index ->
        val log = state.filteredLogs[index]
        val isExpanded = expandedLogId == log.checkedOn

        LogItemCard(
            log = log,
            isExpanded = isExpanded,
            onClick = {
                expandedLogId = if (isExpanded) -1L else log.checkedOn
            },
            onCopyDetails = {
                // Copy functionality
            },
            modifier = Modifier.animateItemPlacement(
                animationSpec = tween(durationMillis = 300)
            )
        )
    }
}

Don't forget to update the function signature of your components to accept a modifier parameter:

@OptIn(ExperimentalLayoutApi::class)
@Composable
private fun ActiveFiltersSection(
    showTriggeredOnly: Boolean,
    alertType: AlertType?,
    notifierType: NotifierType?,
    dateRange: Pair<Long?, Long?>,
    onClearFilter: (String) -> Unit,
    modifier: Modifier = Modifier
) {
    Column(
        modifier = modifier
            .fillMaxWidth()
            .padding(bottom = 8.dp)
            .animateContentSize(
                animationSpec = spring(
                    dampingRatio = Spring.DampingRatioMediumBouncy,
                    stiffness = Spring.StiffnessMedium
                )
            ),
    ) {
        // Existing code...
    }
}

This approach will animate both the addition/removal of the active filters section as well as the position changes of other items when the list content changes.

@hossain-khan hossain-khan marked this pull request as ready for review March 7, 2025 14:03
@hossain-khan hossain-khan merged commit ca38d7f into main Mar 7, 2025
4 checks passed
@hossain-khan hossain-khan deleted the improve-log-screen-by-ai branch March 7, 2025 14:06
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