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

[ABW-2743] Create basic UI for bottom sheet to access factor sources #750

Merged
merged 42 commits into from
Feb 14, 2024

Conversation

giannis-rdx
Copy link
Contributor

@giannis-rdx giannis-rdx commented Jan 15, 2024

Description

This is the first PR of the ABW-2743 ticket and contains

  • a basic UI with its viewmodel and nav graph for the bottom sheet
  • app events to trigger this bottom sheet similar to HandleStatusEvents
  • a new icon for creating account bottom sheet

Another chained PR will follow. The next PR will contain logic for the

  • ABW-2743: Establish communication/access between Factor Sources and UI
  • ABW-2744: creating accounts

How to test

No need to test. The app should run as expected.

Screenshot

In the next PR

PR submission checklist

  • I have ran the app and it works as expected.

@giannis-rdx giannis-rdx self-assigned this Jan 15, 2024
@giannis-rdx giannis-rdx marked this pull request as draft January 15, 2024 18:08
@giannis-rdx giannis-rdx changed the title [ABW-2743] Create basic UI for bottom sheet to access factor sources [ABW-2743] [do not review it] Create basic UI for bottom sheet to access factor sources Jan 15, 2024
@giannis-rdx giannis-rdx force-pushed the task/refactor-current-network-not-nullable branch from 03fa8d1 to 3da3bed Compare January 18, 2024 15:00
@giannis-rdx giannis-rdx changed the base branch from task/refactor-current-network-not-nullable to main January 21, 2024 10:28
@giannis-rdx giannis-rdx force-pushed the feature/ABW-2743-create-basic-ui-for-bottom-sheet branch from 7f89447 to 92399bf Compare January 21, 2024 10:31
@giannis-rdx giannis-rdx changed the title [ABW-2743] [do not review it] Create basic UI for bottom sheet to access factor sources [ABW-2743] Create basic UI for bottom sheet to access factor sources Jan 21, 2024
@giannis-rdx giannis-rdx marked this pull request as ready for review January 21, 2024 10:52
@giannis-rdx giannis-rdx force-pushed the feature/ABW-2743-create-basic-ui-for-bottom-sheet branch from 39212ed to 7dbd00a Compare January 23, 2024 08:58
Copy link
Contributor

@micbakos-rdx micbakos-rdx left a comment

Choose a reason for hiding this comment

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

Nice 👌! I would personally remove the ...BottomSheet notion from the composable's name since, in my opinion is an implementation detail.

@@ -87,6 +87,10 @@ class MainViewModel @Inject constructor(
.events
.filterIsInstance<AppEvent.Status>()

val accessFactorSourceEvents = appEventBus
.events
.filterIsInstance<AppEvent.AccessFactorSource.ToCreateAccount>()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe .filterIsInstance<AppEvent.AccessFactorSource>() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@giannis-rdx
Copy link
Contributor Author

Nice 👌! I would personally remove the ...BottomSheet notion from the composable's name since, in my opinion is an implementation detail.

I am always open for better names!!
But I need a suggestion here because I am running out of ideas. 😄
from AccessFactorSourceBottomSheet to AccessFactorSourceDialog?
AccessFactorSourceScreen but it's not screen?

) {
BottomSheetDialogWrapper(
modifier = modifier,
onDismiss = {
Copy link
Contributor

Choose a reason for hiding this comment

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

lambda not needed here ? can be just onDismiss = onDismiss

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true. will change in the next PR

@giannis-rdx giannis-rdx force-pushed the feature/ABW-2743-create-basic-ui-for-bottom-sheet branch from af736ef to 688feb7 Compare January 24, 2024 17:42
) {
Text(
style = RadixTheme.typography.title,
text = stringResource(id = R.string.derivePublicKeys_titleCreateAccount)
Copy link
Contributor

Choose a reason for hiding this comment

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

this will be parametrized to reflect use context of the dialog?

@giannis-rdx giannis-rdx force-pushed the feature/ABW-2743-create-basic-ui-for-bottom-sheet branch 2 times, most recently from 5376eb4 to e525b75 Compare February 6, 2024 13:32
giannis-rdx and others added 27 commits February 9, 2024 18:24
…sources-create-accounts

[ABW-2744] Access factor sources refactoring - create accounts
Copy link

sonarcloud bot commented Feb 14, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 40%)

See analysis details on SonarCloud

@giannis-rdx giannis-rdx merged commit daad234 into main Feb 14, 2024
8 of 9 checks passed
@giannis-rdx giannis-rdx deleted the feature/ABW-2743-create-basic-ui-for-bottom-sheet branch February 14, 2024 10:07
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants