Skip to content

Update ProfileScreen to properly use ProfileUiState and display Projects count#10

Closed
TheRealAshik wants to merge 23 commits into
masterfrom
feature/update-profile-screen-15598241077014123616
Closed

Update ProfileScreen to properly use ProfileUiState and display Projects count#10
TheRealAshik wants to merge 23 commits into
masterfrom
feature/update-profile-screen-15598241077014123616

Conversation

@TheRealAshik
Copy link
Copy Markdown
Owner

No description provided.

TheRealAshik and others added 23 commits May 15, 2026 13:32
…hardcoding

- Added `HomeScreen` matching the prototype with four sections: My Work, Favorites, Shortcuts, Recent.
- Defined `HomeViewModel` and `HomeUiState` providing placeholder data via `StateFlow`.
- Created `strings.xml` for all strings and `Dimens.kt` for spacing/size tokens.
- Updated `MainScreen` to wire `HomeScreen` into the Home tab with active (filled) and inactive (outlined) icons.
- Updated `AGENTS.md` with strict No Hardcoding Policy.
- Create strings.xml to house string resources
- Create `RepositoryDetailScreen` and `RepositoryDetailUiState` matching prototypes
- Create `PullRequestListScreen` and `PullRequestListUiState`
- Create `CompareBranchSheet` ModalBottomSheet
- Add `Dimens.kt` to extract UI dimensions and enforce "no hardcoding" rules
- Ensure stateless screen implementations
This commit introduces the initial UI implementation for the Inbox
and Explore tabs. It follows strict rules of zero hardcoding:

- Created strings.xml for all texts and mock texts.
- Created Dimensions.kt object for layout sizes.
- Utilized MaterialTheme tokens for colors, typography, and shapes.

Screens are wired up to MainScreen's bottom navigation, correctly switching
between them. Mock data is exposed through StateFlow in ViewModels
(InboxViewModel and ExploreViewModel). UI matches the given design prototype.
- Created stateless Composable screens `ProfileScreen` and `RepositoryListScreen`.
- Created ViewModels (`ProfileViewModel` and `RepositoryListViewModel`) exposing mock UI states with `StateFlow`.
- Created `strings.xml` to hold all UI strings and mock data strings.
- Enforced zero-hardcoding policy by using `MaterialTheme` exclusively for tokens.
- Extracted all dimensions into a new `Dimens.kt` object.
- Replaced hardcoded strings and colors in MainScreen and ProfileScreen with extracted tokens.
- Added crossfade navigation logic mapped from main home screen avatar to profile and then repo list.
…ount sheet

- Created ViewModels and States for settings, code options, and notification options
- Created Compose Multiplatform SettingsScreen, CodeOptionsScreen, NotificationOptionsScreen, and AccountsSheet
- Applied Material3 and MaterialTheme styling and strict no-hardcoding constants using a SettingsTokens class and strings.xml resource values
- GitHubApiClient: add GitHubRepo, GitHubOrg, GitHubNotification models and
  getUserRepos, getUserOrgs, getNotifications, getStarredRepos endpoints
- HomeUiState: replace StringResource-based types with plain-string RepoItem,
  OrgItem, NotificationItem; add Error state
- HomeViewModel: fetch repos/orgs/notifications in parallel via async/await
- HomeScreen: render real data with RepoRow, OrgRow, NotificationRow; add
  error state with retry
- ProfileUiState: rewrite as sealed class (Loading, Error, Success)
- ProfileViewModel: fetch user, repos, orgs, starred in parallel
- ProfileScreen: render real profile data; fix broken IconButton missing Icon call;
  add Loading/Error states with retry
@TheRealAshik TheRealAshik deleted the feature/update-profile-screen-15598241077014123616 branch May 16, 2026 07:36
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly expands the GitHub client application by implementing a comprehensive navigation structure using Navigation Compose, integrating Ktor for API interactions, and adding several new screens including Home, Profile, Inbox, Explore, and Settings. It also establishes a 'No Hardcoding Policy' in the project documentation and provides platform-specific token storage implementations. The review feedback identifies a critical security issue regarding plain-text token storage on Android, incorrect ViewModel instantiation within composables, and violations of the new hardcoding policy. Additionally, there are suggestions to consolidate redundant dimension objects and ensure the UI state correctly reflects the PR's objectives regarding project counts.

Comment on lines +9 to +10
private val prefs: SharedPreferences =
context.getSharedPreferences("github_prefs", Context.MODE_PRIVATE)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

security-high high

Storing Personal Access Tokens (PAT) in plain text using SharedPreferences is insecure. As noted in the TODO, this should be replaced with EncryptedSharedPreferences from the Jetpack Security library to ensure sensitive credentials are encrypted at rest.

}
composable<Route.Profile> {
ProfileScreen(
viewModel = ProfileViewModel(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Directly instantiating ProfileViewModel() inside the composable block is incorrect. This causes a new ViewModel instance to be created on every recomposition or navigation entry, preventing it from being properly scoped to the navigation backstack or surviving configuration changes. Use the viewModel() factory method to correctly manage the ViewModel lifecycle.

Suggested change
viewModel = ProfileViewModel(),
viewModel = androidx.lifecycle.viewmodel.compose.viewModel { ProfileViewModel() },

item { HomeDivider() }

// Organizations
item { SectionHeader(title = "Organizations") }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Hardcoded string "Organizations" violates the "No Hardcoding Policy" defined in AGENTS.md. All user-visible strings must be moved to strings.xml and accessed via stringResource().

Suggested change
item { SectionHeader(title = "Organizations") }
item { SectionHeader(title = stringResource(Res.string.nav_organizations)) }
References
  1. All user-visible strings -> strings.xml + stringResource() (link)


import androidx.compose.ui.unit.dp

object Dimensions {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This file introduces a redundant Dimensions object. A similar Dimens object already exists in dev.therealashik.github.Dimens. To maintain a clean architecture and avoid confusion, consolidate all UI constants into a single source of truth as per the project guidelines.

val publicRepos: Int,
val popularRepos: List<PopularRepo>,
val orgs: List<OrgSummary>,
val starredCount: Int
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The Success state is missing a field for the projects count. Since the PR objective is to 'display Projects count', this field should be added to the UI state and populated by the ViewModel.

        val starredCount: Int,
        val projectsCount: Int

}
},
label = stringResource(Res.string.nav_projects),
count = 0,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The projects count is currently hardcoded to 0. To fulfill the PR objective of displaying the projects count, this value should be retrieved from the ProfileUiState.Success state.

Suggested change
count = 0,
count = state.projectsCount,

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