Open
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
…into fix/FTH-200-ui-issues-in-faith
…into fix/FTH-200-ui-issues-in-faith
mohannadahmed00
requested changes
Dec 2, 2025
...ommonMain/kotlin/net/thechance/mena/faith/presentation/feature/quran/surah/SurahViewModel.kt
Outdated
Show resolved
Hide resolved
faith_presentation/src/commonMain/composeResources/values-ar/strings.xml
Outdated
Show resolved
Hide resolved
faith_presentation/src/commonMain/composeResources/values/strings.xml
Outdated
Show resolved
Hide resolved
...h_data/src/commonMain/kotlin/net/thechance/mena/faith/data/repository/QuranRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
...h_data/src/commonMain/kotlin/net/thechance/mena/faith/data/repository/QuranRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
| ) | ||
| id = id, | ||
| name = name, | ||
| nameAr = name, |
Collaborator
There was a problem hiding this comment.
I think it’s a bad idea to use the same value for name and nameAr, so what’s the difference between them?
Collaborator
Author
There was a problem hiding this comment.
A domain should have only one name, how can u solve this issue in mapping
Collaborator
There was a problem hiding this comment.
The API call returns a List<RecitersRequest>, and since RecitersRequest contains both name and arabicName, it can be mapped directly to ReciterDto without issues. This makes the mapper from Reciter to ReciterDto unnecessary.
mohannadahmed00
requested changes
Dec 2, 2025
...on/src/commonMain/kotlin/net/thechance/mena/faith/presentation/feature/main/MainViewModel.kt
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Removes redundant padding from the
ReciterItemand moves the horizontal padding to the parent screens (ReciterSelectionScreen,SurahRecitersScreen,DownloadedRecitersScreen) for better layout consistency.Adds a background color and clip shape to the
ReciterItemwithin the swipeable card on theDownloadedRecitersScreen.Removes the unnecessary
fillMaxWidthandpaddingmodifiers from theSearchEmptyStatecomposable in theReciterSelectionScreenandDownloadedRecitersScreen.The
SearchEmptyStatenow internally usesfillMaxSizeto handle its own layout, simplifying the call sites.Add
reciterArabicNametoDownlodedSurdomain model and corresponding UI states.Implement
localizedName()extension function to provide the correct reciter name based on the app's language (Arabic or English).Update the data layer, including the repository and mappers, to fetch and handle Arabic reciter names.
Integrate the localization logic into various UI components, including the Quran player, downloaded surah list, and downloaded reciters screen, to display the appropriate name.
Translate the
tilawahTypefrom English to Arabic within theReciterDto.toRecitermapper based on the application's language.handleTilawahfunction to perform the translation.