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

Move padding to summary for larger tap target #38

Merged
merged 2 commits into from Jun 14, 2021

Conversation

benaltair
Copy link
Contributor

@benaltair benaltair commented Jun 11, 2021

When details is closed, the padding on this element makes the box larger than the tap target, since you can only click on summary itself. By moving the padding to summary when details is collapsed, the tap target takes up the entire visual box. Then it can be moved back to details when expanded to keep padding around the contents.

Before:
image
(blue above is the tap target, so clicking on the padded area of details does nothing, even though there's no visual distinction)

After:
image
(entire box is now a tap target, with padding applied to summary element when collapsed)

I suppose it's also possible to use padding: unset or similar for details[open] if you wanted to keep it more general than padding: 0;. Also there are other ways to handle this in terms of CSS attribute selectors I'm sure but this is a start.

Love this library - I recognize a lot has gone into it. Thanks for maintaining!

When `details` is closed, the padding on this element makes the box larger than the tap target, since you can only click on `summary` itself. By moving the padding to `summary` when `details` is collapsed, the tap target takes up the entire visual box. Then it can be moved back to `details` when expanded to keep padding around the contents.

I suppose it's also possible to use `padding: unset` or similar for `details[open]` if you wanted to keep it more general than `padding: 0;`

Love this simple library - I recognize a lot has gone into it. Thanks for maintaining!
@benaltair benaltair marked this pull request as ready for review June 11, 2021 16:26
@kevquirk
Copy link
Owner

Great catch and contribution. Thanks!

@kevquirk kevquirk merged commit 79ee233 into kevquirk:main Jun 14, 2021
@benaltair benaltair deleted the patch-1 branch June 15, 2021 21:28
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.

None yet

2 participants