-
Notifications
You must be signed in to change notification settings - Fork 639
refactor(feature:client): merge dialog ViewModel into main ViewModel in client identifiers #2438
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
refactor(feature:client): merge dialog ViewModel into main ViewModel in client identifiers #2438
Conversation
Note: I am not actively working on this PR at the moment. I have pushed the work done so far in case someone else wants to pick it up and continue from here. Feel free to close this PR once someone picks up the ticket. |
This is almost done, however, can't test it and upload the screen recording since login is not working currently. |
@niyajali I too tested it, and it was working fine except for a state issue — the dialog name and close (×) icon remain visible even after a successful submission. I believe this can be addressed when the new UI/UX mockups are incorporated. If everything looks good, kindly go ahead and merge it. |
Taken from the ticket and pasted here for quick reference: Before Fix: Before_CIVM.webmAfter Fix: After_CIVM.webmOriginally uploaded by @sam-arth07 |
Then close the dialog after form submission in ViewModel @biplab1 |
@@ -152,6 +144,7 @@ internal fun ClientIdentifiersScreen( | |||
actions = { | |||
IconButton( | |||
onClick = { | |||
onShowDialog() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And see here, you're showing the dialog but I'm not seeing any codebase which closes this dialog state.
onDismiss = { showCreateIdentifierDialog = false }, | ||
onIdentifierCreated = { | ||
showCreateIdentifierDialog = false | ||
showCreateSuccessMessage = true | ||
onIdentifierCreated() | ||
reloadIdentifiers() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try this like a before and why don't you handle the Dialog state from ViewModel
showCreateIdentifierDialog = false
Okay, @biplab1 I don't see any difference after watching both videos, can you create a ticket for the issue and all and others will take a look. And I'm merging this now |
…in client identifiers (openMF#2438)
…in client identifiers (openMF#2438)
Fixes - Jira-#496
Screen recording uploaded in the ticket.
Run the static analysis check
./gradlew check
orci-prepush.sh
to make sure you didn't break anythingIf you have multiple commits please combine them into one commit by squashing them.