Skip to content

Conversation

@yesshreyes
Copy link
Collaborator

No description provided.

…e topic name.

- Sort topics alphabetically in `TopicViewModel`.
@yesshreyes yesshreyes requested a review from RajashekarRaju June 5, 2025 13:13
Copy link
Member

@RajashekarRaju RajashekarRaju left a comment

Choose a reason for hiding this comment

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

Computing the first‐character inside the Composable

  • Topic.name is never null or empty, so firstOrNull()?.uppercase() ?: "?" and the “?” fallback aren’t needed.
  • Putting this logic in the UI layer forces the Composable to handle string parsing and placeholder logic.

Solution

 * Keep `data class Topic(val name: String)` unchanged.
 * Create a separate UI model, e.g.:

   ```kotlin
   data class TopicUi(
     val name: String,
     val initial: String,
     val isBookmarked: Boolean
   )
   ```
 * In the ViewModel, after sorting `List<Topic>`, map each `Topic` → `TopicUi` by computing `initial = domain.name.first().uppercase()`.
 * In the Composable, simply use `Text(text = topicUi.initial)`.

UI and domain models are mixed

  • When UI logic (like extracting an initial) lives in the Composable, any change to how you compute or display that character requires touching multiple UI files.

Solution

By introducing TopicUi and mapping inside the ViewModel, all string‐manipulation stays in one place. The UI only displays already‐prepared fields.

…ayer.

- Update `TopicViewModel` to emit `TopicUi` objects, including initial and bookmarked status.
- Modify `TopicCard`, `TopicList`, and `TopicScreenUI` to consume `TopicUi`.
- Update previews to use `TopicUi` and sample data.
@yesshreyes yesshreyes requested a review from RajashekarRaju June 6, 2025 16:45
@yesshreyes yesshreyes requested a review from RajashekarRaju June 7, 2025 18:48
@yesshreyes yesshreyes merged commit b5fc0c5 into master Jun 7, 2025
1 check passed
@RajashekarRaju RajashekarRaju deleted the shreyas/topic-card-icon-update branch June 9, 2025 14:48
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.

3 participants