Skip to content

Ux improvements #362

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

Merged
merged 3 commits into from
Mar 6, 2025
Merged

Ux improvements #362

merged 3 commits into from
Mar 6, 2025

Conversation

hossain-khan
Copy link
Owner

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

This pull request includes several changes to the CurrentAlertListScreen.kt file and adds a new drawable resource. The most important changes include the addition of new imports, modifications to the CurrentWeatherAlertPresenter class, updates to the AlertListItem function, and changes to the CurrentWeatherAlertsPreview function.

Changes to CurrentAlertListScreen.kt:

  • Imports:

    • Added imports for Box, height, and serviceConfig to support new layout and configuration functionalities. [1] [2]
    • Removed the unnecessary import of BuildConfig.
  • CurrentWeatherAlertPresenter class:

    • Simplified the forecastSourceName assignment by using the serviceConfig extension function.
  • AlertListItem function:

    • Updated the comment to reflect the new purpose of showing the forecast source.
    • Enhanced the layout to include an icon and additional padding for better visual separation.
  • CurrentWeatherAlertsPreview function:

    • Changed the isAlertActive property to true for better preview representation.

New drawable resource:

  • info_24dp.xml:
    • Added a new vector drawable resource for the info icon used in the AlertListItem function.

I think the trailing content with icon is a waste of space for list item.
Also when the alert is not active, the happy face doesn't make sense to all users.

I want to refactor such that the warning icon overlaps with leading content icon.

----

I'll refactor the code to overlay the warning icon on top of the leading content icon when an alert is active, and remove the happy face icon completely when it's not active. This will save space in the list item.

Here's the implementation:

```kotlin
leadingContent = {
    Box(contentAlignment = Alignment.Center) {
        // Main category icon
        Icon(
            painter = painterResource(iconResId),
            contentDescription = null,
            tint = MaterialTheme.colorScheme.primary,
            modifier = Modifier.size(48.dp),
        )

        // Conditionally show warning icon as an overlay when alert is active
        if (data.isAlertActive) {
            Icon(
                painter = painterResource(R.drawable.warning_24dp),
                contentDescription = "Alert active",
                tint = MaterialTheme.colorScheme.error,
                modifier = Modifier
                    .size(24.dp)
                    .align(Alignment.BottomEnd)
            )
        }
    }
},
// Remove the trailingContent completely
```

This approach:
1. Uses a Box to layer the warning icon over the category icon
2. Shows the warning icon only when an alert is active
3. Positions the warning icon at the bottom-right of the category icon
4. Uses error color for the warning icon to make it stand out
5. Completely removes the trailing content and the happy face icon

You'll need to import `Box` and `contentAlignment` if not already imported:
```kotlin
import androidx.compose.foundation.layout.Box
```
@hossain-khan hossain-khan self-assigned this Mar 6, 2025
@Copilot Copilot AI review requested due to automatic review settings March 6, 2025 15:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

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

@hossain-khan hossain-khan enabled auto-merge March 6, 2025 15:24
@hossain-khan hossain-khan merged commit 55aaf32 into main Mar 6, 2025
5 checks passed
@hossain-khan hossain-khan deleted the ux-improvements branch March 6, 2025 15:26
Copy link

github-actions bot commented Mar 6, 2025

Code Coverage Report

View Coverage Report

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.

1 participant