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

[ProfileCardScreen] Design enhancement #971

Conversation

masah517
Copy link
Contributor

@masah517 masah517 commented Sep 5, 2024

Issue

Overview (Required)

  • Added following features according to the issue(please also refer to the screenshots below)
    • Back navigation icon
    • Clear icon placed for profile screen's EditText(clear function not yet implemented)

Links

Screenshot (Optional if screenshot test is present or unrelated to UI)

Before After
iOS before iOS after

Movie (Optional)

Before After

@@ -260,6 +260,7 @@ private fun NavGraphBuilder.mainScreen(
profileCardScreen(
contentPadding = contentPadding,
onClickShareProfileCard = externalNavController::onShareProfileCardClick,
onNavigationBackIconClick = mainNestedNavController::navigateUp,
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for your pull request! I think we need to change IosComposeKaigiApp as well 🙏

Copy link
Contributor Author

@masah517 masah517 Sep 5, 2024

Choose a reason for hiding this comment

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

Thanks for the prompt review.
Seems like IosComposeKaigiApp and unitTest needs to be sync with the added code.

I will fix and resubmit the code asap.

Copy link

github-actions bot commented Sep 6, 2024

Snapshot diff report

File name Image
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is exists - when cli
ck edit button - it
should show edit scr
een]_compare.png
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is does not exists -
when url contains s
ub domain - it shoul
d not show error mes
sage]_compare.png
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is does not exists -
when url top level
domain is missing -
it should show error
message]_compare.pn
g
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is exists - when cli
ck edit button - if
all required fields
are filled in - it s
hould make sure the
Create button is act
ivated]_compare.png
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is does not exists -
when url contains I
DN domain name - it
should not show erro
r message]_compare.p
ng
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is exists - when cli
ck edit button - whe
n if a required fiel
d has not been fille
d in - it should mak
e sure the Create bu
tton is deactivated]
_compare.png
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is does not exists -
when protocol is mi
ssing - it should no
t show error message
]_compare.png
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is does not exists -
it should show edit
screen]_compare.png
ProfileCardScreenTes
t[ProfileCardScreen
- when profile card
is does not exists -
when url protocol i
s invalid - it shoul
d show error message
]_compare.png

@takahirom
Copy link
Member

Sorry, I forgot about this. We should implement back button only when they already have a profile card and are editing it.
And there is a issue and it is assigned 😇
I think you can remove the back button in this pr.
#811

@masah517
Copy link
Contributor Author

masah517 commented Sep 9, 2024

Hi @takahirom -san

My apologies for accidentally closing the PR due to the force push, while resolving the conflict.
Could you please continue the review of this PR at here

Sorry for any problem occurred

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.

2 participants