Skip to content

Fix: Improve spacing and styling for 'Try Indexed Search' button #4388

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

Open
wants to merge 4 commits into
base: release/4.0
Choose a base branch
from

Conversation

ajay020
Copy link

@ajay020 ajay020 commented Apr 15, 2025

Description

Improved layout and styling of the 'Try Indexed Search!' button to match expected UI guidelines.

  • Aligned button vertically with surrounding text
  • Added ripple effect on touch
  • Fixed spacing and visual distinction
  • Used proper layout structure to ensure responsiveness

Before

issue

After

Tablet

Screenshot 2025-04-15 111752

Phone

Screenshot 2025-04-15 111635

Issue tracker

Fixes #4332

Automatic tests

  • Added test cases
    (Only UI/layout changes, so no automatic tests added)

Manual tests

  • Done

  • Device: Motorola e13

  • OS: Android 13

Build tasks success

Successfully running following tasks on local:

  • ./gradlew assembleDebug
  • ./gradlew spotlessCheck

@VishnuSanal VishnuSanal added the pr-awaiting-initial-review this PR is awaiting for an initial review label Apr 15, 2025
Copy link
Member

@VishnuSanal VishnuSanal left a comment

Choose a reason for hiding this comment

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

@ajay020 thanks, appreciate your interest in contributing to amaze! but, the UX in this PR deviates from the expected UX (https://github.com/orgs/TeamAmaze/discussions/3984). an option for indexed search never appears (edit: okay, it was not about the behaviour, t'was just about the display string, I presume). please take a look, thanks.

@VishnuSanal VishnuSanal added pr-requested-changes this PR is awaiting an update from the author and removed pr-awaiting-initial-review this PR is awaiting for an initial review labels Apr 18, 2025
@ajay020
Copy link
Author

ajay020 commented Apr 19, 2025

@VishnuSanal Thanks for the review! You're right — this PR only changes the display string and layout styling, not the actual behaviour.

I went through the discussion, but I'm still unclear on what part might be deviating from the expected UX. Could you please clarify what you meant by "an option for indexed search never appears"?

@ajay020
Copy link
Author

ajay020 commented Apr 19, 2025

@VishnuSanal I realised I hadn’t updated the button text string as expected. Just pushed a fix to address that. Let me know if you'd prefer any other change.

Copy link
Member

@VishnuSanal VishnuSanal left a comment

Choose a reason for hiding this comment

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

this is better! but, I have some suggestions, PTAL at the commments

@ajay020 ajay020 force-pushed the fix/indexed-search-button-spacing branch from 236a93b to c73581f Compare April 19, 2025 10:03
@ajay020
Copy link
Author

ajay020 commented Apr 19, 2025

In the layout file, I’ve reduced the internal padding and added a margin so that the text “Not finding what you're looking for” and the button “Try index search” now appear on a single line.

phone preview

Screenshot 2025-04-19 152151

tablet preview

Screenshot 2025-04-19 151358

On smaller screens, the button text wraps into two lines due to limited space. If that doesn’t look good to you, I can try vertically stacking the text and the button instead. Let me know what you'd prefer!

Copy link
Member

@VishnuSanal VishnuSanal left a comment

Choose a reason for hiding this comment

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

LGTM, good work! please address slight alignment issues, and we're golden, thx! :)

android:id="@+id/searchDeepSearchTV"
android:layout_width="wrap_content"
<LinearLayout
android:id="@+id/deepSearchContainer"
Copy link
Member

@VishnuSanal VishnuSanal Apr 21, 2025

Choose a reason for hiding this comment

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

I would prefer the same behaviour from the tablet layout here too.

i.e, to make the whole container fill width, and align both notFindingTextView & tryDeepSearchButton at both ends where the remaining space is filled. i.e, to right-align tryDeepSearchButton.

but that's personal preference. let's wait for other maintainers to chime in :)

Copy link
Member

@VishnuSanal VishnuSanal left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 awesome work! :)

@VishnuSanal VishnuSanal added pr-awaiting-final-review this PR is awaiting a final review/approval and removed pr-requested-changes this PR is awaiting an update from the author labels Apr 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-awaiting-final-review this PR is awaiting a final review/approval
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insufficient Spacing Causes 'Try Indexed Search!' Button to appear as plain text
2 participants