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

ANDROID-15138 Search Input in compose #380

Merged
merged 11 commits into from
Sep 10, 2024

Conversation

yamal-alm
Copy link
Contributor

@yamal-alm yamal-alm commented Sep 6, 2024

🥅 What's the goal?

Search Input in compose as defined in Mistica specs:
Captura de pantalla 2024-09-06 a las 12 32 00

🚧 How do we do it?

  • Create a new SearchInput composable similar to other existing input implementation.
  • Add a leading icon with the magnifying glass and a trailing "X" icon button with its corresponding clear input action
  • Add content descriptors for "clear search" button. They are the same defined in mistica-web
  • Add composable to catalog

☑️ Checks

  • I updated the documentation, including readmes and wikis. If this is a breaking change, tag the PR with "Breaking Change" label and remember to include breaking change migration guide in release notes where this version is released.
  • Tested with dark mode.
  • Tested with API 24.
  • Sync done with iOS team for this feature to ensure alignment, if applies.

🧪 How can I test this?

  • 🖼️ Screenshots/Videos
  • Mistica App QR or download link
  • Reviewed by Mistica design team
Grabacion.de.pantalla.2024-09-10.a.las.10.06.53.mov

Copy link

github-actions bot commented Sep 6, 2024

📱 New catalog for testing generated: Download

},
trailingIcon = {
IconButton(onClick = {
onValueChange("")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if user clicks here, we reset the entered text by invoking onValueChange lambda with an empty string

@yamal-alm yamal-alm requested review from a team, dpastor, jeprubio and AnaMontes11 and removed request for a team September 6, 2024 10:53
Copy link

github-actions bot commented Sep 6, 2024

📱 New catalog for testing generated: Download

Copy link

github-actions bot commented Sep 6, 2024

📱 New catalog for testing generated: Download

Copy link
Contributor

@jeprubio jeprubio left a comment

Choose a reason for hiding this comment

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

Great 👏

Copy link
Contributor

@dpastor dpastor left a comment

Choose a reason for hiding this comment

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

👍

android:viewportHeight="24">
<path
android:pathData="M16.841,17.853a0.722,0.722 0,0 0,0.948 -1.086L13.022,12l4.766,-4.767 0.065,-0.074a0.722,0.722 0,0 0,-1.086 -0.947L12,10.978 7.233,6.211l-0.074,-0.064a0.722,0.722 0,0 0,-0.947 1.086L10.979,12 6.21,16.767l-0.064,0.074a0.722,0.722 0,0 0,1.086 0.947L12,13.022l4.767,4.767z"
android:fillColor="#000000"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

generated screenshots doesn't render these two icons, I guess it is because I was using theme attributes and the roborazzi test doesn't set any xml theme before rendering the composable. I'm using a hardcoded color just to verify that that is the problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it seems that the problem is that we are not setting any xml theme during roborazzi test and these type of attributes are not resolved.

Copy link

github-actions bot commented Sep 6, 2024

📱 New catalog for testing generated: Download

Copy link

github-actions bot commented Sep 9, 2024

📱 New catalog for testing generated: Download

@AnaMontes11
Copy link

Hi @yamal-alm, could we add the helper text?

Copy link

📱 New catalog for testing generated: Download

@yamal-alm
Copy link
Contributor Author

Hi @yamal-alm, could we add the helper text?

yes! helper text added both in catalog and in screenshot baseline

Copy link

@AnaMontes11 AnaMontes11 left a comment

Choose a reason for hiding this comment

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

Lgtm!

@yamal-alm yamal-alm merged commit 6ec1d54 into main Sep 10, 2024
@yamal-alm yamal-alm deleted the android-15138/compose_search_input branch September 10, 2024 08:20
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.

4 participants