Skip to content
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

feat(feature:auth): migrated to cmp #2772

Merged
merged 3 commits into from
Feb 13, 2025

Conversation

Nagarjuna0033
Copy link
Contributor

@Nagarjuna0033 Nagarjuna0033 commented Feb 11, 2025

Fixes - Jira-#120

Didn't create a Jira ticket, click here to create new.

Please Add Screenshots If there are any UI changes.

Before After
MM_compressed.mp4
desk_compressed.mp4

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the static analysis check ./gradlew check or ci-prepush.sh to make sure you didn't break anything

  • If you have multiple commits please combine them into one commit by squashing them.

@Nagarjuna0033
Copy link
Contributor Author

Nagarjuna0033 commented Feb 11, 2025

@niyajali , I have used getString for stringResources if this approach is right, we can merge this PR

@niyajali
Copy link
Collaborator

@Nagarjuna0033 Upload video for other platforms like desktop, wasmJS if it's working fine

@niyajali niyajali added the firebase-test-on Upload Application on Firebase For Testing label Feb 12, 2025
@Nagarjuna0033
Copy link
Contributor Author

@Nagarjuna0033 Upload video for other platforms like desktop, wasmJS if it's working fine

Uploaded for desktop and for web the issue still persists we will fix in other PR

Copy link
Collaborator

@niyajali niyajali left a comment

Choose a reason for hiding this comment

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

@Nagarjuna0033 Quickly fix the following changes and let me know, we will proceed with merging this PR

@Nagarjuna0033
Copy link
Contributor Author

@Nagarjuna0033 Quickly fix the following changes and let me know, we will proceed with merging this PR

@niyajali I have done changes and for web we can't use stringResources because it is not supported yet

@HekmatullahAmin
Copy link
Contributor

@niyajali I have done changes and for web we can't use stringResources because it is not supported yet

@Nagarjuna0033 ,

I saw your message about stringResource not being supported in Web. Could you clarify what you mean by that?

From what I understand, you can have stringResource(Res.string....) inside the common module, which should be accessible by all platforms, including Web. If you're referring to formatted strings, I've attached a piece of code that successfully uses stringResource in the common module to handle formatted strings like:

<string name="feature_account_string_and_string">%1$s %2$s</string>
Screenshot 2025-02-12 at 17 53 19

@Nagarjuna0033
Copy link
Contributor Author

Nagarjuna0033 commented Feb 12, 2025

@niyajali I have done changes and for web we can't use stringResources because it is not supported yet

@Nagarjuna0033 ,

I saw your message about stringResource not being supported in Web. Could you clarify what you mean by that?

From what I understand, you can have stringResource(Res.string....) inside the common module, which should be accessible by all platforms, including Web. If you're referring to formatted strings, I've attached a piece of code that successfully uses stringResource in the common module to handle formatted strings like:

<string name="feature_account_string_and_string">%1$s %2$s</string>
Screenshot 2025-02-12 at 17 53 19

@HekmatullahAmin we can use stringResource in commonMain but I am talking about using in cmp-web module which we want to use stringResource in cmp-web/src/wasmJsMain/kotlin/Main.kt these file. We can only use stringResource in Composable functions right. so in that file when marking fun main() it showing it is not supported

@niyajali
Copy link
Collaborator

@Nagarjuna0033 No, It should be supported on web too, create a composeResource/string/string.xml like before and run
generateResourceAccessorsForJsMain or generateResourceAccessorsForWasmMain

@niyajali niyajali merged commit e596618 into openMF:kmp-impl Feb 13, 2025
7 checks passed
revanthkumarJ pushed a commit to revanthkumarJ/mifos-mobile that referenced this pull request Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
firebase-test-on Upload Application on Firebase For Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants