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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 5 commits into from
Jun 17, 2024
Merged

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

merged 5 commits into from
Jun 17, 2024
+38 −5

Conversation

DBowen33
Copy link
Contributor

@DBowen33 DBowen33 commented May 23, 2024 •
edited

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

@DBowen33
fix(material/paginator): items per page form field touch target issue
f98f733 
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
fix(material/paginator): added transparent element for larger touch t…
d1d85b2 
…arget

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

Fixes b/225394124
@DBowen33
fix(material/paginator): add new line
c1a283b 
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
@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 11, 2024 •
edited

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

@DBowen33
fix(material/paginator): added density tokens
717c3b6 
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
@DBowen33
fix(material/paginator): fix lint error
07f55a8 
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
@DBowen33 @andrewseguin
fix(material/paginator): items per page form field touch target size …
bad8f6a 
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crisbeto crisbeto crisbeto approved these changes

@andrewseguin andrewseguin andrewseguin approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DBowen33 @andrewseguin @crisbeto