Skip to content

fix: Increase input clear button target size to 24×24 pixels#3253

Merged
avinashbot merged 1 commit intomainfrom
button-touch-target
Feb 11, 2025
Merged

fix: Increase input clear button target size to 24×24 pixels#3253
avinashbot merged 1 commit intomainfrom
button-touch-target

Conversation

@avinashbot
Copy link
Member

@avinashbot avinashbot commented Feb 5, 2025

Description

Not sold on this yet, but it does work. We don't have to update all "inline-icon" buttons, just the ones that are within 24px of another interactive control. In this case, the clear button is on top of the input element, so we're literally 0px away from the nearest interactive control.

Related links, issue #, if available: AWSUI-60212

How has this been tested?

Should be visually identical. Just pushed to my dev branch, and everything looks good.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

  • Changes include appropriate documentation updates.
  • Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
  • Changes do not include unsupported browser features, see CONTRIBUTING.md.
  • Changes were manually tested for accessibility, see accessibility guidelines.

Security

Testing

  • Changes are covered with new/existing unit tests?
  • Changes are covered with new/existing integration tests?

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@codecov
Copy link

codecov bot commented Feb 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.44%. Comparing base (89e8c18) to head (08434bd).
Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3253   +/-   ##
=======================================
  Coverage   96.44%   96.44%           
=======================================
  Files         791      791           
  Lines       22569    22570    +1     
  Branches     7794     7803    +9     
=======================================
+ Hits        21766    21767    +1     
  Misses        750      750           
  Partials       53       53           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@avinashbot avinashbot marked this pull request as ready for review February 6, 2025 11:26
@avinashbot avinashbot requested a review from a team as a code owner February 6, 2025 11:26
@avinashbot avinashbot requested review from orangevolon and removed request for a team February 6, 2025 11:26
'disabled-background': transparent,
'disabled-border-color': transparent,
'disabled-color': awsui.$color-text-button-inline-icon-disabled,
),
Copy link
Member

Choose a reason for hiding this comment

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

I think introducing a new variant to the button would make it a bit confusing for the customers especially with a name like pointer-target which makes sense to us, but not necessarily to the user.

Why are we even using inline variant of the button inside the input? It's not inline with text at all. Alternatively we could introduce something like block-icon which is 24x24 and is a block. Since the icon variant also 24x20.

Copy link
Member Author

Choose a reason for hiding this comment

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

This variant is internal button only. So it isn't exposed to users, just to the input component.

Copy link
Member Author

Choose a reason for hiding this comment

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

(Just to answer your second question, there is a more standard inline-block icon variant, but the visuals are completely different and the paddings+borders are much heavier than just 24x24.)

@avinashbot avinashbot added this pull request to the merge queue Feb 11, 2025
Merged via the queue into main with commit 7c8f26d Feb 11, 2025
38 checks passed
@avinashbot avinashbot deleted the button-touch-target branch February 11, 2025 15:24
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