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

fix(material/paginator): items per page form field touch target size insufficient #29109

Merged
merged 5 commits into from
Jun 17, 2024

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
…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
23 of 26 checks passed
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