Skip to content

Conversation

DBowen33
Copy link
Contributor

@DBowen33 DBowen33 commented May 23, 2024

Items per page form field touch target does not have sufficient touch target size of 48 x 48 pixels.
Changed from 84 x 41 to 84 x 48 pixels.

Before:
image

After:
image

fixes b/225394124

items per page form field touch target does not have sufficient touch target
size (48 x 48)

fixes b/202731532
@DBowen33 DBowen33 marked this pull request as ready for review May 23, 2024 19:03
@DBowen33 DBowen33 requested a review from crisbeto as a code owner May 23, 2024 19:03
DBowen33 added 2 commits May 29, 2024 17:06
…arget

added transparent touch target element on top of paginator select  to get touch target to 48 pixel height

Fixes b/225394124
add new line

fixes b/225394124
@DBowen33 DBowen33 requested a review from crisbeto May 29, 2024 18:40
@andrewseguin andrewseguin added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Jun 11, 2024
Copy link

github-actions bot commented Jun 11, 2024

Deployed dev-app for 07f55a8 to: https://ng-dev-previews-comp--pr-angular-components-29109-dev-dq7gxpzg.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

@andrewseguin
Copy link
Contributor

The touch target escapes the bounds of the paginator when the density is -3 and lower. Other components adjust for this by having a token that determines whether the touch target is displayed. Can we do something similar here?

Code for reference: https://github.com/search?q=repo%3Aangular%2Fcomponents%20touch-target-display&type=code

added density tokens for paginator touch target

Fixes b/225394124
@DBowen33 DBowen33 requested a review from a team as a code owner June 12, 2024 14:33
@DBowen33 DBowen33 requested review from amysorto and andrewseguin and removed request for a team June 12, 2024 14:33
fix lint error

fixes #225394124
@DBowen33
Copy link
Contributor Author

The touch target escapes the bounds of the paginator when the density is -3 and lower. Other components adjust for this by having a token that determines whether the touch target is displayed. Can we do something similar here?

Code for reference: https://github.com/search?q=repo%3Aangular%2Fcomponents%20touch-target-display&type=code

Added density tokens to paginator
https://screencast.googleplex.com/cast/NTQ0MTk5NjI4MDgyMzgwOHxiOTVmYTRmYS0zYg

@andrewseguin andrewseguin added action: merge The PR is ready for merge by the caretaker target: patch This PR is targeted for the next patch release labels Jun 17, 2024
@andrewseguin andrewseguin removed the request for review from amysorto June 17, 2024 19:44
@andrewseguin andrewseguin merged commit 99b3312 into angular:main Jun 17, 2024
andrewseguin pushed a commit that referenced this pull request Jun 17, 2024
…insufficient (#29109)

* fix(material/paginator): items per page form field touch target issue

items per page form field touch target does not have sufficient touch target
size (48 x 48)

fixes b/202731532

* fix(material/paginator): added transparent element for larger touch target

added transparent touch target element on top of paginator select  to get touch target to 48 pixel height

Fixes b/225394124

* fix(material/paginator): add new line

add new line

fixes b/225394124

* fix(material/paginator): added density tokens

added density tokens for paginator touch target

Fixes b/225394124

* fix(material/paginator): fix lint error

fix lint error

fixes #225394124

(cherry picked from commit 99b3312)
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker dev-app preview When applied, previews of the dev-app are deployed to Firebase target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants